[v5] rs6000: Use vector addition when left shifting by 1[PR119702]
Commit Message
Hi,
Following is version 5 of the patch proposed for master to fix PR119702.
This patch has been bootstrapped and regtested on powerpc64le-linux.
Kindly review the patch.
Thanks and regards,
Avinash Jayakar
Changes from v4:
1. Added comments for the new predicate "vector_constant_1".
2. Added new tests for altivec vector types.
3. Added comments in test file.
Changes from v3:
1. Add author information before changelog.
2. Right placement of PR target/119702.
3. Added new test to check multiply by 2 generates vadd insn.
Changes from v2:
1. Indentation fixes in the commit message
2. define_insn has the name *altivec_vsl<VI_char>_const_1
3. Iterate starting from 0 for checking vector constant = 1 and
fixed source code formatting for the for loop.
4. Removed unused macro in pr119702-1.c test file
Changes from v1:
1. Use define_insn instead of define_expand to recognize left
shift by constant 1 and generate add instruction.
2. Updated test cases to cover integer types from byte, half-
word, word and double word.
rs6000: Use vector addition when left shifting by 1[PR119702]
Whenever a vector of integers is left shifted by a constant value 1,
gcc generates the following code for powerpc64le target:
vspltisw 0,1
vsld 2,2,0
Instead the following code can be generated which is more efficient:
vaddudm 2,2,2
This patch adds a pattern in altivec.md to recognize a vector left
shift by a constant value, and generates an add instruction if constant
is 1.
Added the pattern in altivec.md to recognize a vector left shift by a
constant value, and generate add instructions if constant is 1.
Added a predicate in predicates.md to recognize if the rtl node is a
uniform constant vector with value 1.
Two test files have been added
1. Check if left shift by 1 is converted to vadd instructions
for byte, half-word, word, and double-word.
2. Check the same but for multiply by 2.
3. 2 more test cases using altivec vector types
2025-09-17 Avinash Jayakar <avinashd@linux.ibm.com>
gcc/ChangeLog:
PR target/119702
* config/rs6000/altivec.md (*altivec_vsl<VI_char>_const_1): recognize
<< 1 and replace with vadd insn.
* config/rs6000/predicates.md (vector_constant_1): predicate to check
vector constant of all 1.
gcc/testsuite/ChangeLog:
PR target/119702
* gcc.target/powerpc/pr119702-1.c: New test
* gcc.target/powerpc/pr119702-2.c: New test
* gcc.target/powerpc/pr119702-3.c: New test
* gcc.target/powerpc/pr119702-4.c: New test
---
gcc/config/rs6000/altivec.md | 8 +++
gcc/config/rs6000/predicates.md | 12 ++++
gcc/testsuite/gcc.target/powerpc/pr119702-1.c | 57 +++++++++++++++++++
gcc/testsuite/gcc.target/powerpc/pr119702-2.c | 57 +++++++++++++++++++
gcc/testsuite/gcc.target/powerpc/pr119702-3.c | 36 ++++++++++++
gcc/testsuite/gcc.target/powerpc/pr119702-4.c | 36 ++++++++++++
6 files changed, 206 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr119702-1.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr119702-2.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr119702-3.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr119702-4.c
Comments
On 17/09/25 2:10 pm, Avinash Jayakar wrote:
> Hi,
>
> Following is version 5 of the patch proposed for master to fix PR119702.
> This patch has been bootstrapped and regtested on powerpc64le-linux.
> Kindly review the patch.
>
> Thanks and regards,
> Avinash Jayakar
>
> Changes from v4:
> 1. Added comments for the new predicate "vector_constant_1".
> 2. Added new tests for altivec vector types.
So the current gcc code does not handle altivec vector types too?
> 3. Added comments in test file.
> Changes from v3:
> 1. Add author information before changelog.
> 2. Right placement of PR target/119702.
> 3. Added new test to check multiply by 2 generates vadd insn.
> Changes from v2:
> 1. Indentation fixes in the commit message
> 2. define_insn has the name *altivec_vsl<VI_char>_const_1
The name for define_insn has been removed in this patch. Please add it
back.
> diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
> index 7edc288a656..4fa34864ed3 100644
> --- a/gcc/config/rs6000/altivec.md
> +++ b/gcc/config/rs6000/altivec.md
> @@ -2107,6 +2107,14 @@
> "vsrv %0,%1,%2"
> [(set_attr "type" "vecsimple")])
>
> +(define_insn ""
> + [(set (match_operand:VI2 0 "register_operand" "=v")
> + (ashift:VI2 (match_operand:VI2 1 "register_operand" "v")
> + (match_operand:VI2 2 "vector_constant_1" "")))]
> + "<VI_unit>"
> + "vaddu<VI_char>m %0,%1,%1"
> +)
> +
> (define_insn "*altivec_vsl<VI_char>"
> [(set (match_operand:VI2 0 "register_operand" "=v")
> (ashift:VI2 (match_operand:VI2 1 "register_operand" "v")
> diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
> index 647e89afb6a..c3e40890673 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -924,6 +924,18 @@
> }
> })
>
> +;; Return 1 if the operand is a vector constant with 1 in all of the elements.
> +(define_predicate "vector_constant_1"
> + (match_code "const_vector")
> +{
> + unsigned nunits = GET_MODE_NUNITS (mode), i;
> + for (i = 0; i < nunits; i++) {
> + if (INTVAL (CONST_VECTOR_ELT(op, i)) != 1)
> + return 0;
> + }
> + return 1;
> +})
> +
> ;; Return 1 if operand is 0.0.
> (define_predicate "zero_fp_constant"
> (and (match_code "const_double")> diff --git a/gcc/testsuite/gcc.target/powerpc/pr119702-4.c b/gcc/testsuite/gcc.target/powerpc/pr119702-4.c
> new file mode 100644
> index 00000000000..1f745451c0a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr119702-4.c
> @@ -0,0 +1,36 @@
> +/* { dg-do compile { target lp64 } } */
lp64 is not required. The ISA does not say that the vaddu<x>m instructions
are only for lp64 mode.
> +/* { dg-options "-O2 -maltivec -mdejagnu-cpu=power10" } */
Why do we need power10 here? And if you are specifying power10, there is no
need to specify -maltivec.
Same comments for the test pr119702-3.c too.
Surya
On 19/09/25 9:09 am, Avinash Jayakar wrote:
> On Thu, 2025-09-18 at 21:46 +0530, Surya Kumari Jangala wrote:
>>
>>
>> On 17/09/25 2:10 pm, Avinash Jayakar wrote:
>>> Hi,
>>>
>>> Following is version 5 of the patch proposed for master to fix
>>> PR119702.
>>> This patch has been bootstrapped and regtested on powerpc64le-
>>> linux.
>>> Kindly review the patch.
>>>
>>> Thanks and regards,
>>> Avinash Jayakar
>>>
>>> Changes from v4:
>>> 1. Added comments for the new predicate
>>> "vector_constant_1".
>>> 2. Added new tests for altivec vector types.
>>
>> So the current gcc code does not handle altivec vector types too?
>>
>>> 3. Added comments in test file.
>>> Changes from v3:
>>> 1. Add author information before changelog.
>>> 2. Right placement of PR target/119702.
>>> 3. Added new test to check multiply by 2 generates vadd
>>> insn.
>>> Changes from v2:
>>> 1. Indentation fixes in the commit message
>>> 2. define_insn has the name *altivec_vsl<VI_char>_const_1
>>
>>
>> The name for define_insn has been removed in this patch. Please add
>> it
>> back.
>>
>>
>>> diff --git a/gcc/config/rs6000/altivec.md
>>> b/gcc/config/rs6000/altivec.md
>>> index 7edc288a656..4fa34864ed3 100644
>>> --- a/gcc/config/rs6000/altivec.md
>>> +++ b/gcc/config/rs6000/altivec.md
>>> @@ -2107,6 +2107,14 @@
>>> "vsrv %0,%1,%2"
>>> [(set_attr "type" "vecsimple")])
>>>
>>> +(define_insn ""
>>> + [(set (match_operand:VI2 0 "register_operand" "=v")
>>> + (ashift:VI2 (match_operand:VI2 1 "register_operand" "v")
>>> + (match_operand:VI2 2 "vector_constant_1"
>>> "")))]
>>> + "<VI_unit>"
>>> + "vaddu<VI_char>m %0,%1,%1"
>>> +)
>>> +
>>> (define_insn "*altivec_vsl<VI_char>"
>>> [(set (match_operand:VI2 0 "register_operand" "=v")
>>> (ashift:VI2 (match_operand:VI2 1 "register_operand" "v")
>>> diff --git a/gcc/config/rs6000/predicates.md
>>> b/gcc/config/rs6000/predicates.md
>>> index 647e89afb6a..c3e40890673 100644
>>> --- a/gcc/config/rs6000/predicates.md
>>> +++ b/gcc/config/rs6000/predicates.md
>>> @@ -924,6 +924,18 @@
>>> }
>>> })
>>>
>>> +;; Return 1 if the operand is a vector constant with 1 in all of
>>> the elements.
>>> +(define_predicate "vector_constant_1"
>>> + (match_code "const_vector")
>>> +{
>>> + unsigned nunits = GET_MODE_NUNITS (mode), i;
>>> + for (i = 0; i < nunits; i++) {
>>> + if (INTVAL (CONST_VECTOR_ELT(op, i)) != 1)
>>> + return 0;
>>> + }
>>> + return 1;
>>> +})
>>> +
>>> ;; Return 1 if operand is 0.0.
>>> (define_predicate "zero_fp_constant"
>>> (and (match_code "const_double")> diff --git
>>> a/gcc/testsuite/gcc.target/powerpc/pr119702-4.c
>>> b/gcc/testsuite/gcc.target/powerpc/pr119702-4.c
>>> new file mode 100644
>>> index 00000000000..1f745451c0a
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr119702-4.c
>>> @@ -0,0 +1,36 @@
>>> +/* { dg-do compile { target lp64 } } */
>>
>> lp64 is not required. The ISA does not say that the vaddu<x>m
>> instructions
>> are only for lp64 mode.
>>
>>
>>> +/* { dg-options "-O2 -maltivec -mdejagnu-cpu=power10" } */
>> Why do we need power10 here? And if you are specifying power10, there
>> is no
>> need to specify -maltivec.
>>
> Can I just use power10, actually there is an issue in power8, for
> double word, operand #1(value to be shifted by) instead of using a
> splat immediate, the generated code loads from memory(TOC) as below
> 0: addis 2,12,.TOC.-.LCF0@ha
> 15 addi 2,2,.TOC.-.LCF0@l
> 1 .localentry lshift1_64,.-lshift1_64
> 2 addis 9,2,.LC0@toc@ha
> 3 addi 9,9,.LC0@toc@l
> 4 lvx 0,0,9
> 5 vsld 2,2,0
> Instead it should just have been
> 39 vspltisw 1,1
> 1 vsld 0,0,1
> So this is only the case for power8, and I believe there must be a PR
> for it similar to the one jeevitha is working on?
What is the code generated for this testcase w/o your patch for P10
and P8?
Surya
On Tue, 2025-09-23 at 20:10 +0530, Surya Kumari Jangala wrote:
>
>
>
> What is the code generated for this testcase w/o your patch for P10
> and P8?
>
After looking into it a bit further I did find an issue which can be
fixed for left shift case, but not sure how to do it for multiplication
case.
For left shift case, the original code is
vector unsigned long long
lshift1_64 (vector unsigned long long a)
{
return a << (vector unsigned long long) { 1, 1 };
}
Since the constant is one, the check in function
vspltisw_vupkhsw_constant_p returned false, but it should have returned
false for -1 as per the commit message. Correcting that value has fixed
this test case, and no other regression in ppc64le. So I can send that
as second patch in this sequence or separately.
But for this test case
vector unsigned long long
lshift1_64 (vector unsigned long long a, vector unsigned long long b)
{
return a * (vector unsigned long long) { 2, 2 };
}
The issue still exists, because the pattern recognition which is done
in vect_pattern_recog is not done (the question that I have asked in
gcc mailing list).
Before the fix for p8
xxpermdi 0,34,34,3
mfvsrd 9,34
mfvsrd 10,0
sldi 9,9,1
mtvsrd 0,9
sldi 10,10,1
mtvsrd 34,10
xxpermdi 34,0,34,0
blr
For p10
vspltisw v0,1
vsld v2,v2,v0
blr
After the fix for p8 the code is same and in p10 we get vaddudm.
The veclower pass scalarized the vector function since it does not find
a optab for mulv2di3 for power8, and by the time we are in expand pass
there is no vector code.
Thanks and regards,
Avinash Jayakar
@@ -2107,6 +2107,14 @@
"vsrv %0,%1,%2"
[(set_attr "type" "vecsimple")])
+(define_insn ""
+ [(set (match_operand:VI2 0 "register_operand" "=v")
+ (ashift:VI2 (match_operand:VI2 1 "register_operand" "v")
+ (match_operand:VI2 2 "vector_constant_1" "")))]
+ "<VI_unit>"
+ "vaddu<VI_char>m %0,%1,%1"
+)
+
(define_insn "*altivec_vsl<VI_char>"
[(set (match_operand:VI2 0 "register_operand" "=v")
(ashift:VI2 (match_operand:VI2 1 "register_operand" "v")
@@ -924,6 +924,18 @@
}
})
+;; Return 1 if the operand is a vector constant with 1 in all of the elements.
+(define_predicate "vector_constant_1"
+ (match_code "const_vector")
+{
+ unsigned nunits = GET_MODE_NUNITS (mode), i;
+ for (i = 0; i < nunits; i++) {
+ if (INTVAL (CONST_VECTOR_ELT(op, i)) != 1)
+ return 0;
+ }
+ return 1;
+})
+
;; Return 1 if operand is 0.0.
(define_predicate "zero_fp_constant"
(and (match_code "const_double")
new file mode 100644
@@ -0,0 +1,57 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -maltivec" } */
+/* { dg-require-effective-target powerpc_altivec } */
+
+/* PR target/119702 -- verify left shift by 1 is converted into
+ a vaddu<x>m instruction. */
+
+#include <inttypes.h>
+
+void lshift1_64(uint64_t *a) {
+ a[0] <<= 1;
+ a[1] <<= 1;
+}
+
+void lshift1_32(unsigned int *a) {
+ a[0] <<= 1;
+ a[1] <<= 1;
+ a[2] <<= 1;
+ a[3] <<= 1;
+}
+
+void lshift1_16(uint16_t *a) {
+ a[0] <<= 1;
+ a[1] <<= 1;
+ a[2] <<= 1;
+ a[3] <<= 1;
+ a[4] <<= 1;
+ a[5] <<= 1;
+ a[6] <<= 1;
+ a[7] <<= 1;
+}
+
+void lshift1_8(uint8_t *a) {
+ a[0] <<= 1;
+ a[1] <<= 1;
+ a[2] <<= 1;
+ a[3] <<= 1;
+ a[4] <<= 1;
+ a[5] <<= 1;
+ a[6] <<= 1;
+ a[7] <<= 1;
+ a[8] <<= 1;
+ a[9] <<= 1;
+ a[10] <<= 1;
+ a[11] <<= 1;
+ a[12] <<= 1;
+ a[13] <<= 1;
+ a[14] <<= 1;
+ a[15] <<= 1;
+}
+
+
+/* { dg-final { scan-assembler-times {\mvaddudm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvadduwm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvadduhm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvaddubm\M} 1 } } */
+
new file mode 100644
@@ -0,0 +1,57 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -maltivec" } */
+/* { dg-require-effective-target powerpc_altivec } */
+
+/* PR target/119702 -- verify multiply by 2 is converted into
+ a vaddu<x>m instruction. */
+
+#include <inttypes.h>
+
+void lshift1_64(uint64_t *a) {
+ a[0] *= 2;
+ a[1] *= 2;
+}
+
+void lshift1_32(unsigned int *a) {
+ a[0] *= 2;
+ a[1] *= 2;
+ a[2] *= 2;
+ a[3] *= 2;
+}
+
+void lshift1_16(uint16_t *a) {
+ a[0] *= 2;
+ a[1] *= 2;
+ a[2] *= 2;
+ a[3] *= 2;
+ a[4] *= 2;
+ a[5] *= 2;
+ a[6] *= 2;
+ a[7] *= 2;
+}
+
+void lshift1_8(uint8_t *a) {
+ a[0] *= 2;
+ a[1] *= 2;
+ a[2] *= 2;
+ a[3] *= 2;
+ a[4] *= 2;
+ a[5] *= 2;
+ a[6] *= 2;
+ a[7] *= 2;
+ a[8] *= 2;
+ a[9] *= 2;
+ a[10] *= 2;
+ a[11] *= 2;
+ a[12] *= 2;
+ a[13] *= 2;
+ a[14] *= 2;
+ a[15] *= 2;
+}
+
+
+/* { dg-final { scan-assembler-times {\mvaddudm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvadduwm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvadduhm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvaddubm\M} 1 } } */
+
new file mode 100644
@@ -0,0 +1,36 @@
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -maltivec -mdejagnu-cpu=power10" } */
+/* { dg-require-effective-target powerpc_altivec } */
+
+/* PR target/119702 -- verify vector shift left by 1 is converted into
+ a vaddu<x>m instruction. */
+
+vector unsigned long long
+lshift1_64 (vector unsigned long long a)
+{
+ return a << (vector unsigned long long) { 1, 1 };
+}
+
+vector unsigned int
+lshift1_32 (vector unsigned int a)
+{
+ return a << (vector unsigned int) { 1, 1, 1, 1 };
+}
+
+vector unsigned short
+lshift1_16 (vector unsigned short a)
+{
+ return a << (vector unsigned short) { 1, 1, 1, 1, 1, 1, 1, 1 };
+}
+
+vector unsigned char
+lshift1_8 (vector unsigned char a)
+{
+ return a << (vector unsigned char) { 1, 1, 1, 1, 1, 1, 1, 1,
+ 1, 1, 1, 1, 1, 1, 1, 1 };
+}
+
+/* { dg-final { scan-assembler-times {\mvaddudm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvadduwm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvadduhm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvaddubm\M} 1 } } */
new file mode 100644
@@ -0,0 +1,36 @@
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -maltivec -mdejagnu-cpu=power10" } */
+/* { dg-require-effective-target powerpc_altivec } */
+
+/* PR target/119702 -- verify vector multiply by 2 is converted into
+ a vaddu<x>m instruction. */
+
+vector unsigned long long
+lshift1_64 (vector unsigned long long a)
+{
+ return a * (vector unsigned long long) { 2, 2 };
+}
+
+vector unsigned int
+lshift1_32 (vector unsigned int a)
+{
+ return a * (vector unsigned int) { 2, 2, 2, 2 };
+}
+
+vector unsigned short
+lshift1_16 (vector unsigned short a)
+{
+ return a * (vector unsigned short) { 2, 2, 2, 2, 2, 2, 2, 2 };
+}
+
+vector unsigned char
+lshift1_8 (vector unsigned char a)
+{
+ return a * (vector unsigned char) { 2, 2, 2, 2, 2, 2, 2, 2,
+ 2, 2, 2, 2, 2, 2, 2, 2 };
+}
+
+/* { dg-final { scan-assembler-times {\mvaddudm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvadduwm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvadduhm\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvaddubm\M} 1 } } */