* [PATCH V3] rs6000: Don't ICE when compiling the __builtin_vsx_splat_2di built-in [PR113950]
@ 2024-03-03 18:55 jeevitha
2024-03-06 9:27 ` Kewen.Lin
0 siblings, 1 reply; 6+ messages in thread
From: jeevitha @ 2024-03-03 18:55 UTC (permalink / raw)
To: GCC Patches, Kewen.Lin, Segher Boessenkool, Michael Meissner,
Peter Bergner
Hi All,
The following patch has been bootstrapped and regtested on powerpc64le-linux.
When we expand the __builtin_vsx_splat_2di function, we were allowing immediate
value for second operand which causes an unrecognizable insn ICE. Even though
the immediate value was forced into a register, it wasn't correctly assigned
to the second operand. So corrected the assignment of op1 to operands[1].
2024-02-29 Jeevitha Palanisamy <jeevitha@linux.ibm.com>
gcc/
PR target/113950
* config/rs6000/vsx.md (vsx_splat_<mode>): Corrected assignment to
operand1.
gcc/testsuite/
PR target/113950
* gcc.target/powerpc/pr113950.c: New testcase.
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index 6111cc90eb7..f135fa079bd 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -4666,8 +4666,8 @@
rtx op1 = operands[1];
if (MEM_P (op1))
operands[1] = rs6000_force_indexed_or_indirect_mem (op1);
- else if (!REG_P (op1))
- op1 = force_reg (<VSX_D:VEC_base>mode, op1);
+ else
+ operands[1] = force_reg (<VSX_D:VEC_base>mode, op1);
})
(define_insn "vsx_splat_<mode>_reg"
diff --git a/gcc/testsuite/gcc.target/powerpc/pr113950.c b/gcc/testsuite/gcc.target/powerpc/pr113950.c
new file mode 100644
index 00000000000..64566a580d9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr113950.c
@@ -0,0 +1,24 @@
+/* PR target/113950 */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-O1 -mvsx" } */
+
+/* Verify we do not ICE on the following. */
+
+void abort (void);
+
+int main ()
+{
+ int i;
+ vector signed long long vsll_result, vsll_expected_result;
+ signed long long sll_arg1;
+
+ sll_arg1 = 300;
+ vsll_expected_result = (vector signed long long) {300, 300};
+ vsll_result = __builtin_vsx_splat_2di (sll_arg1);
+
+ for (i = 0; i < 2; i++)
+ if (vsll_result[i] != vsll_expected_result[i])
+ abort();
+
+ return 0;
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3] rs6000: Don't ICE when compiling the __builtin_vsx_splat_2di built-in [PR113950]
2024-03-03 18:55 [PATCH V3] rs6000: Don't ICE when compiling the __builtin_vsx_splat_2di built-in [PR113950] jeevitha
@ 2024-03-06 9:27 ` Kewen.Lin
2024-03-15 20:34 ` Peter Bergner
0 siblings, 1 reply; 6+ messages in thread
From: Kewen.Lin @ 2024-03-06 9:27 UTC (permalink / raw)
To: jeevitha; +Cc: GCC Patches, Segher Boessenkool, Michael Meissner, Peter Bergner
Hi,
on 2024/3/4 02:55, jeevitha wrote:
> Hi All,
>
> The following patch has been bootstrapped and regtested on powerpc64le-linux.
>
> When we expand the __builtin_vsx_splat_2di function, we were allowing immediate
> value for second operand which causes an unrecognizable insn ICE. Even though
> the immediate value was forced into a register, it wasn't correctly assigned
> to the second operand. So corrected the assignment of op1 to operands[1].
>
> 2024-02-29 Jeevitha Palanisamy <jeevitha@linux.ibm.com>
>
> gcc/
> PR target/113950
> * config/rs6000/vsx.md (vsx_splat_<mode>): Corrected assignment to
> operand1.
Nit: s/Corrected/Correct/, maybe add "and simplify else if with else.".
>
> gcc/testsuite/
> PR target/113950
> * gcc.target/powerpc/pr113950.c: New testcase.
>
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 6111cc90eb7..f135fa079bd 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -4666,8 +4666,8 @@
> rtx op1 = operands[1];
> if (MEM_P (op1))
> operands[1] = rs6000_force_indexed_or_indirect_mem (op1);
> - else if (!REG_P (op1))
> - op1 = force_reg (<VSX_D:VEC_base>mode, op1);
> + else
> + operands[1] = force_reg (<VSX_D:VEC_base>mode, op1);
> })
>
> (define_insn "vsx_splat_<mode>_reg"
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr113950.c b/gcc/testsuite/gcc.target/powerpc/pr113950.c
> new file mode 100644
> index 00000000000..64566a580d9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr113950.c
> @@ -0,0 +1,24 @@
> +/* PR target/113950 */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-O1 -mvsx" } */
Nit: s/-O1/-O2/, -O2 is preferred when the failure can be reproduced
with -O2 (not just -O1), since optimization may change the level in
which it's placed, -O2 is more general.
As the discussions in the thread of the previous versions, I think
Segher agreed this approach, so OK for trunk with the above nits
tweaked, thanks!
BR,
Kewen
> +
> +/* Verify we do not ICE on the following. */
> +
> +void abort (void);
> +
> +int main ()
> +{
> + int i;
> + vector signed long long vsll_result, vsll_expected_result;
> + signed long long sll_arg1;
> +
> + sll_arg1 = 300;
> + vsll_expected_result = (vector signed long long) {300, 300};
> + vsll_result = __builtin_vsx_splat_2di (sll_arg1);
> +
> + for (i = 0; i < 2; i++)
> + if (vsll_result[i] != vsll_expected_result[i])
> + abort();
> +
> + return 0;
> +}
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3] rs6000: Don't ICE when compiling the __builtin_vsx_splat_2di built-in [PR113950]
2024-03-06 9:27 ` Kewen.Lin
@ 2024-03-15 20:34 ` Peter Bergner
2024-03-18 1:30 ` Kewen.Lin
0 siblings, 1 reply; 6+ messages in thread
From: Peter Bergner @ 2024-03-15 20:34 UTC (permalink / raw)
To: Kewen.Lin, jeevitha; +Cc: GCC Patches, Segher Boessenkool, Michael Meissner
On 3/6/24 3:27 AM, Kewen.Lin wrote:
> on 2024/3/4 02:55, jeevitha wrote:
>> The following patch has been bootstrapped and regtested on powerpc64le-linux.
>>
>> When we expand the __builtin_vsx_splat_2di function, we were allowing immediate
>> value for second operand which causes an unrecognizable insn ICE. Even though
>> the immediate value was forced into a register, it wasn't correctly assigned
>> to the second operand. So corrected the assignment of op1 to operands[1].
[snip]
> As the discussions in the thread of the previous versions, I think
> Segher agreed this approach, so OK for trunk with the above nits
> tweaked, thanks!
The bogus vsx_splat_<mode> code goes all the way back to GCC 8, so we
should backport this fix. Segher and Ke Wen, can we get an approval
to backport this to all the open release branches (GCC 13, 12, 11)?
Thanks.
Jeevitha, once we get approval, please perform the backports.
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3] rs6000: Don't ICE when compiling the __builtin_vsx_splat_2di built-in [PR113950]
2024-03-15 20:34 ` Peter Bergner
@ 2024-03-18 1:30 ` Kewen.Lin
2024-04-17 9:05 ` jeevitha
0 siblings, 1 reply; 6+ messages in thread
From: Kewen.Lin @ 2024-03-18 1:30 UTC (permalink / raw)
To: Peter Bergner, jeevitha; +Cc: GCC Patches, Segher Boessenkool, Michael Meissner
Hi,
on 2024/3/16 04:34, Peter Bergner wrote:
> On 3/6/24 3:27 AM, Kewen.Lin wrote:
>> on 2024/3/4 02:55, jeevitha wrote:
>>> The following patch has been bootstrapped and regtested on powerpc64le-linux.
>>>
>>> When we expand the __builtin_vsx_splat_2di function, we were allowing immediate
>>> value for second operand which causes an unrecognizable insn ICE. Even though
>>> the immediate value was forced into a register, it wasn't correctly assigned
>>> to the second operand. So corrected the assignment of op1 to operands[1].
> [snip]
>> As the discussions in the thread of the previous versions, I think
>> Segher agreed this approach, so OK for trunk with the above nits
>> tweaked, thanks!
>
> The bogus vsx_splat_<mode> code goes all the way back to GCC 8, so we
> should backport this fix. Segher and Ke Wen, can we get an approval
> to backport this to all the open release branches (GCC 13, 12, 11)?
> Thanks.
Sure, okay for backporting this to all active branches, thanks!
>
> Jeevitha, once we get approval, please perform the backports.
>
> Peter
>
>
BR,
Kewen
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3] rs6000: Don't ICE when compiling the __builtin_vsx_splat_2di built-in [PR113950]
2024-03-18 1:30 ` Kewen.Lin
@ 2024-04-17 9:05 ` jeevitha
2024-04-17 9:28 ` Kewen.Lin
0 siblings, 1 reply; 6+ messages in thread
From: jeevitha @ 2024-04-17 9:05 UTC (permalink / raw)
To: Kewen.Lin, Peter Bergner, jeevitha
Cc: GCC Patches, Segher Boessenkool, Michael Meissner
Hi,
On 18/03/24 7:00 am, Kewen.Lin wrote:
>> The bogus vsx_splat_<mode> code goes all the way back to GCC 8, so we
>> should backport this fix. Segher and Ke Wen, can we get an approval
>> to backport this to all the open release branches (GCC 13, 12, 11)?
>> Thanks.
>
> Sure, okay for backporting this to all active branches, thanks!
>
I need clarification regarding the backporting of PR113950 to GCC 12.
We encountered an issue while resolving merge conflicts in GCC 12. The
problem lies in extra deletions in the diff due to cherry-picking. Now,
we're unsure about the best approach for handling the backport.
To provide context, I have included the relevant diff snippet below,
diff --cc gcc/config/rs6000/vsx.md
index c45794fb9ed,f135fa079bd..00000000000
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@@ -4562,8 -4666,8 +4562,8 @@@
rtx op1 = operands[1];
if (MEM_P (op1))
operands[1] = rs6000_force_indexed_or_indirect_mem (op1);
- else if (!REG_P (op1))
- op1 = force_reg (<VSX_D:VS_scalar>mode, op1);
+ else
- operands[1] = force_reg (<VSX_D:VEC_base>mode, op1);
++ operands[1] = force_reg (<VSX_D:VS_scalar>mode, op1);
})
I'm seeking your advice on how to proceed with the backport. Do you
think the above change is acceptable, or should we also backport Segher's
commit e0e3ce634818b83965b87512938490df4d57f81d, which caused the conflict?.
There was no regression with both of these changes.
Jeevitha.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3] rs6000: Don't ICE when compiling the __builtin_vsx_splat_2di built-in [PR113950]
2024-04-17 9:05 ` jeevitha
@ 2024-04-17 9:28 ` Kewen.Lin
0 siblings, 0 replies; 6+ messages in thread
From: Kewen.Lin @ 2024-04-17 9:28 UTC (permalink / raw)
To: jeevitha; +Cc: GCC Patches, Segher Boessenkool, Michael Meissner, Peter Bergner
Hi,
on 2024/4/17 17:05, jeevitha wrote:
> Hi,
>
> On 18/03/24 7:00 am, Kewen.Lin wrote:
>
>>> The bogus vsx_splat_<mode> code goes all the way back to GCC 8, so we
>>> should backport this fix. Segher and Ke Wen, can we get an approval
>>> to backport this to all the open release branches (GCC 13, 12, 11)?
>>> Thanks.
>>
>> Sure, okay for backporting this to all active branches, thanks!
>>
>
> I need clarification regarding the backporting of PR113950 to GCC 12.
>
> We encountered an issue while resolving merge conflicts in GCC 12. The
> problem lies in extra deletions in the diff due to cherry-picking. Now,
> we're unsure about the best approach for handling the backport.
>
> To provide context, I have included the relevant diff snippet below,
>
> diff --cc gcc/config/rs6000/vsx.md
> index c45794fb9ed,f135fa079bd..00000000000
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@@ -4562,8 -4666,8 +4562,8 @@@
> rtx op1 = operands[1];
> if (MEM_P (op1))
> operands[1] = rs6000_force_indexed_or_indirect_mem (op1);
> - else if (!REG_P (op1))
> - op1 = force_reg (<VSX_D:VS_scalar>mode, op1);
> + else
> - operands[1] = force_reg (<VSX_D:VEC_base>mode, op1);
> ++ operands[1] = force_reg (<VSX_D:VS_scalar>mode, op1);
> })
>
> I'm seeking your advice on how to proceed with the backport. Do you
> think the above change is acceptable, or should we also backport Segher's
> commit e0e3ce634818b83965b87512938490df4d57f81d, which caused the conflict?.
I prefer the former, which is the least modification, for release branches
let's introduce as few changes as possible, and the amendment on the conflict
is minor and straightforward.
BR,
Kewen
> There was no regression with both of these changes.
>
> Jeevitha.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-17 9:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-03 18:55 [PATCH V3] rs6000: Don't ICE when compiling the __builtin_vsx_splat_2di built-in [PR113950] jeevitha
2024-03-06 9:27 ` Kewen.Lin
2024-03-15 20:34 ` Peter Bergner
2024-03-18 1:30 ` Kewen.Lin
2024-04-17 9:05 ` jeevitha
2024-04-17 9:28 ` Kewen.Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).