Re: RFA[powerpc]: patch to fix PR79916
On 04/13/2018 05:26 PM, Segher Boessenkool wrote:
Hi!
On Fri, Apr 13, 2018 at 04:43:02PM -0400, Vladimir Makarov wrote:
On 04/13/2018 03:58 PM, Alexander Monakov wrote:
Here's another compact variant:
regno = reg_renumber[regno];
if (regno < 0)
regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
Thanks, Alexander. I like your variant more.
Me too :-)
--- testsuite/gcc.target/powerpc/pr79916.c (nonexistent)
+++ testsuite/gcc.target/powerpc/pr79916.c (working copy)
@@ -0,0 +1,556 @@
+/* { dg-do compile { target { powerpc64le-*-* } } } */
Is the testcase specific to LE? If not, please use powerpc*-*-*, or
powerpc*-*-* && lp64 if it needs 64-bit.
I've just tried powerpc64 BE and powerpc. The bug is reproducible
everywhere. So I change it to powerpc*-*-*
+/* { dg-options "-Idfp -fno-expensive-optimizations --param
ira-max-conflict-table-size=0 -mno-popcntd -O3" } */
I think you can drop the -Idfp ? If that option would do anything we
have bigger problems ;-)
I removed it.
Looks fine to me otherwise. Thanks for working on this!
Thank you for the quick review.
I committed the patch to trunk (rev. 259379).
The final variant is in the attachment.
Index: ChangeLog
===
--- ChangeLog (revision 259378)
+++ ChangeLog (working copy)
@@ -1,3 +1,10 @@
+2018-04-13 Vladimir Makarov
+
+ PR rtl-optimization/79916
+ * config/rs6000/rs6000.c (rs6000_emit_move): Use assigned hard
+ regs (if any) to define how to gnerate SD moves when LRA is in
+ progress.
+
2018-04-13 Jakub Jelinek
PR rtl-optimization/85393
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 259378)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2018-04-13 Vladimir Makarov
+
+ PR rtl-optimization/79916
+ * gcc.target/powerpc/pr79916.c: New.
+
2018-04-13 Jakub Jelinek
PR rtl-optimization/85393
Index: config/rs6000/rs6000.c
===
--- config/rs6000/rs6000.c (revision 259330)
+++ config/rs6000/rs6000.c (working copy)
@@ -10610,7 +10610,9 @@ rs6000_emit_move (rtx dest, rtx source,
if (regno >= FIRST_PSEUDO_REGISTER)
{
cl = reg_preferred_class (regno);
- regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
+ regno = reg_renumber[regno];
+ if (regno < 0)
+ regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
}
if (regno >= 0 && ! FP_REGNO_P (regno))
{
@@ -10635,7 +10637,9 @@ rs6000_emit_move (rtx dest, rtx source,
{
cl = reg_preferred_class (regno);
gcc_assert (cl != NO_REGS);
- regno = ira_class_hard_regs[cl][0];
+ regno = reg_renumber[regno];
+ if (regno < 0)
+ regno = ira_class_hard_regs[cl][0];
}
if (FP_REGNO_P (regno))
{
@@ -10664,7 +10668,9 @@ rs6000_emit_move (rtx dest, rtx source,
if (regno >= FIRST_PSEUDO_REGISTER)
{
cl = reg_preferred_class (regno);
- regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][0];
+ regno = reg_renumber[regno];
+ if (regno < 0)
+ regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][0];
}
if (regno >= 0 && ! FP_REGNO_P (regno))
{
@@ -10689,7 +10695,9 @@ rs6000_emit_move (rtx dest, rtx source,
{
cl = reg_preferred_class (regno);
gcc_assert (cl != NO_REGS);
- regno = ira_class_hard_regs[cl][0];
+ regno = reg_renumber[regno];
+ if (regno < 0)
+ regno = ira_class_hard_regs[cl][0];
}
if (FP_REGNO_P (regno))
{
Index: testsuite/gcc.target/powerpc/pr79916.c
===
--- testsuite/gcc.target/powerpc/pr79916.c (nonexistent)
+++ testsuite/gcc.target/powerpc/pr79916.c (working copy)
@@ -0,0 +1,556 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-options "-fno-expensive-optimizations --param ira-max-conflict-table-size=0 -mno-popcntd -O3" } */
+
+#define PASTE2(A,B) A ## B
+#define PASTE(A,B) PASTE2(A,B)
+
+#define TESTVAL_NEG(VAL,SUF,SIZE) \
+ x = PASTE(PASTE(VAL,.),SUF); \
+ si = VAL;\
+ sll = PASTE(VAL,LL); \
+ a = si;\
+ b = sll;\
+ c = VAL;\
+ d = PASTE(VAL,LL); \
+ if ((__builtin_memcmp ((void *)&x, (void *)&a, SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (void *)&b,SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (void *)&c,SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (void *)&d,SIZE) != 0)) \
+__builtin_abort ();
+
+#define TESTVAL_NEG_BIG(VAL,SUF,SIZE) \
+ x = PASTE(PASTE(VAL,.),SUF); \
+ sll = PASTE(VAL,LL); \
+ a = sll;\
+ b = PASTE(VAL,LL); \
+ if ((__builtin_memcmp ((void *)&x, (void *)&a, SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (void *)&b,SIZE) != 0)) \
+__builtin_abort ();
+
+#define TESTVAL_NONNEG(VAL,SUF,SIZE) \
+ x = PASTE(PASTE(VAL,.),SUF); \
+ si = V
Re: RFA[powerpc]: patch to fix PR79916
Hi!
On Fri, Apr 13, 2018 at 04:43:02PM -0400, Vladimir Makarov wrote:
> On 04/13/2018 03:58 PM, Alexander Monakov wrote:
> >Here's another compact variant:
> >
> > regno = reg_renumber[regno];
> > if (regno < 0)
> > regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
> >
> Thanks, Alexander. I like your variant more.
Me too :-)
> --- testsuite/gcc.target/powerpc/pr79916.c(nonexistent)
> +++ testsuite/gcc.target/powerpc/pr79916.c(working copy)
> @@ -0,0 +1,556 @@
> +/* { dg-do compile { target { powerpc64le-*-* } } } */
Is the testcase specific to LE? If not, please use powerpc*-*-*, or
powerpc*-*-* && lp64 if it needs 64-bit.
> +/* { dg-options "-Idfp -fno-expensive-optimizations --param
> ira-max-conflict-table-size=0 -mno-popcntd -O3" } */
I think you can drop the -Idfp ? If that option would do anything we
have bigger problems ;-)
Looks fine to me otherwise. Thanks for working on this!
Segher
Re: RFA[powerpc]: patch to fix PR79916
On 04/13/2018 03:58 PM, Alexander Monakov wrote:
On Fri, 13 Apr 2018, Jakub Jelinek wrote:
if (reg_renumber[regno] >= 0)
regno = reg_renumber[regno];
else
regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
or
regno = (reg_renumber[regno] >= 0
? reg_renumber[regno]
: cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1]);
is better, the latter is perhaps more compact.
Here's another compact variant:
regno = reg_renumber[regno];
if (regno < 0)
regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
Thanks, Alexander. I like your variant more.
Sorry for forgetting GNU standard (it might happen when you are working
on a few different projects simultaneously).
The revised patch is in the attachment.
Index: ChangeLog
===
--- ChangeLog (revision 259376)
+++ ChangeLog (working copy)
@@ -1,3 +1,10 @@
+2018-04-13 Vladimir Makarov
+
+ PR rtl-optimization/79916
+ * config/rs6000/rs6000.c (rs6000_emit_move): Use assigned hard
+ regs (if any) to define how to gnerate SD moves when LRA is in
+ progress.
+
2018-04-13 Jan Hubicka
Bin Cheng
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 259376)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2018-04-13 Vladimir Makarov
+
+ PR rtl-optimization/79916
+ * gcc.target/powerpc/pr79916.c: New.
+
2018-04-13 Andrey Belevantsev
PR rtl-optimization/83852
Index: config/rs6000/rs6000.c
===
--- config/rs6000/rs6000.c (revision 259330)
+++ config/rs6000/rs6000.c (working copy)
@@ -10610,7 +10610,9 @@ rs6000_emit_move (rtx dest, rtx source,
if (regno >= FIRST_PSEUDO_REGISTER)
{
cl = reg_preferred_class (regno);
- regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
+ regno = reg_renumber[regno];
+ if (regno < 0)
+ regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
}
if (regno >= 0 && ! FP_REGNO_P (regno))
{
@@ -10635,7 +10637,9 @@ rs6000_emit_move (rtx dest, rtx source,
{
cl = reg_preferred_class (regno);
gcc_assert (cl != NO_REGS);
- regno = ira_class_hard_regs[cl][0];
+ regno = reg_renumber[regno];
+ if (regno < 0)
+ regno = ira_class_hard_regs[cl][0];
}
if (FP_REGNO_P (regno))
{
@@ -10664,7 +10668,9 @@ rs6000_emit_move (rtx dest, rtx source,
if (regno >= FIRST_PSEUDO_REGISTER)
{
cl = reg_preferred_class (regno);
- regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][0];
+ regno = reg_renumber[regno];
+ if (regno < 0)
+ regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][0];
}
if (regno >= 0 && ! FP_REGNO_P (regno))
{
@@ -10689,7 +10695,9 @@ rs6000_emit_move (rtx dest, rtx source,
{
cl = reg_preferred_class (regno);
gcc_assert (cl != NO_REGS);
- regno = ira_class_hard_regs[cl][0];
+ regno = reg_renumber[regno];
+ if (regno < 0)
+ regno = ira_class_hard_regs[cl][0];
}
if (FP_REGNO_P (regno))
{
Index: testsuite/gcc.target/powerpc/pr79916.c
===
--- testsuite/gcc.target/powerpc/pr79916.c (nonexistent)
+++ testsuite/gcc.target/powerpc/pr79916.c (working copy)
@@ -0,0 +1,556 @@
+/* { dg-do compile { target { powerpc64le-*-* } } } */
+/* { dg-options "-Idfp -fno-expensive-optimizations --param ira-max-conflict-table-size=0 -mno-popcntd -O3" } */
+
+#define PASTE2(A,B) A ## B
+#define PASTE(A,B) PASTE2(A,B)
+
+#define TESTVAL_NEG(VAL,SUF,SIZE) \
+ x = PASTE(PASTE(VAL,.),SUF); \
+ si = VAL;\
+ sll = PASTE(VAL,LL); \
+ a = si;\
+ b = sll;\
+ c = VAL;\
+ d = PASTE(VAL,LL); \
+ if ((__builtin_memcmp ((void *)&x, (void *)&a, SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (void *)&b,SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (void *)&c,SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (void *)&d,SIZE) != 0)) \
+__builtin_abort ();
+
+#define TESTVAL_NEG_BIG(VAL,SUF,SIZE) \
+ x = PASTE(PASTE(VAL,.),SUF); \
+ sll = PASTE(VAL,LL); \
+ a = sll;\
+ b = PASTE(VAL,LL); \
+ if ((__builtin_memcmp ((void *)&x, (void *)&a, SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (void *)&b,SIZE) != 0)) \
+__builtin_abort ();
+
+#define TESTVAL_NONNEG(VAL,SUF,SIZE) \
+ x = PASTE(PASTE(VAL,.),SUF); \
+ si = VAL;\
+ ui = VAL;\
+ sll = PASTE(VAL,LL); \
+ ull = PASTE(VAL,ULL); \
+ a = si;\
+ b = sll;\
+ c = ui;\
+ d = ull;\
+ e = VAL;\
+ f = VAL;\
+ g = PASTE(VAL,LL); \
+ h = PASTE(VAL,ULL); \
+ if ((__builtin_memcmp ((void *)&x, (void *)&a, SIZE) != 0) \
+ || (__builtin_memcmp ((void *)&x, (
Re: RFA[powerpc]: patch to fix PR79916
On Fri, 13 Apr 2018, Jakub Jelinek wrote: > if (reg_renumber[regno] >= 0) > regno = reg_renumber[regno]; > else > regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1]; > or > regno = (reg_renumber[regno] >= 0 > ? reg_renumber[regno] > : cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1]); > is better, the latter is perhaps more compact. Here's another compact variant: regno = reg_renumber[regno]; if (regno < 0) regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1]; Alexander
Re: RFA[powerpc]: patch to fix PR79916
On Fri, Apr 13, 2018 at 03:29:47PM -0400, Vladimir Makarov wrote:
> The attached patch fixes
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79916
>
> The PR is about LRA cycling on some tests when SD data are used. The
> problem was in that actual assigned reg to pseudo was not in the pseudo
> preferred class and this resulted in wrong generated code which LRA tried to
> change again and again.
>
> The patch was successfully bootstrapped and tested on ppc64 (gcc110).
>
> Is it ok to commit?
I have just formatting nits and will defer the actual review to the PowerPC
maintainers. The nit is that all the lines are too long.
Not sure if
if (reg_renumber[regno] >= 0)
regno = reg_renumber[regno];
else
regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
or
regno = (reg_renumber[regno] >= 0
? reg_renumber[regno]
: cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1]);
is better, the latter is perhaps more compact.
> --- config/rs6000/rs6000.c(revision 259330)
> +++ config/rs6000/rs6000.c(working copy)
> @@ -10610,7 +10610,7 @@ rs6000_emit_move (rtx dest, rtx source,
>if (regno >= FIRST_PSEUDO_REGISTER)
> {
> cl = reg_preferred_class (regno);
> - regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][1];
> + regno = reg_renumber[regno] >= 0 ? reg_renumber[regno] : cl ==
> NO_REGS ? -1 : ira_class_hard_regs[cl][1];
> }
>if (regno >= 0 && ! FP_REGNO_P (regno))
> {
> @@ -10635,7 +10635,7 @@ rs6000_emit_move (rtx dest, rtx source,
> {
> cl = reg_preferred_class (regno);
> gcc_assert (cl != NO_REGS);
> - regno = ira_class_hard_regs[cl][0];
> + regno = reg_renumber[regno] >= 0 ? reg_renumber[regno] :
> ira_class_hard_regs[cl][0];
> }
>if (FP_REGNO_P (regno))
> {
> @@ -10664,7 +10664,7 @@ rs6000_emit_move (rtx dest, rtx source,
>if (regno >= FIRST_PSEUDO_REGISTER)
> {
> cl = reg_preferred_class (regno);
> - regno = cl == NO_REGS ? -1 : ira_class_hard_regs[cl][0];
> + regno = reg_renumber[regno] >= 0 ? reg_renumber[regno] : cl ==
> NO_REGS ? -1 : ira_class_hard_regs[cl][0];
> }
>if (regno >= 0 && ! FP_REGNO_P (regno))
> {
> @@ -10689,7 +10689,7 @@ rs6000_emit_move (rtx dest, rtx source,
> {
> cl = reg_preferred_class (regno);
> gcc_assert (cl != NO_REGS);
> - regno = ira_class_hard_regs[cl][0];
> + regno = reg_renumber[regno] >= 0 ? reg_renumber[regno] :
> ira_class_hard_regs[cl][0];
> }
>if (FP_REGNO_P (regno))
> {
Jakub
