Re: [PATCH, GCC] PR target/86487: fix the way 'uses_hard_regs_p' handles paradoxical subregs

[Andre Vieira (lists)]

Hi Vlad,

On 13/02/2019 16:46, Vladimir Makarov wrote:

On 2019-02-13 5:54 a.m., Andre Vieira (lists) wrote:
PING.

Since Jeff is away can another maintainer have a look at this please?


I see the following patch

Yeah I uploaded the wrong patch... sorry. See attached, including a 
testcase, currently only fails on GCC-8 and previous though.

diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 
c061093ed699620afe2dfda60d58066d6967523a..736b084acc552b75ff4d369b6584bc9ab422e21b 
100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -1761,11 +1761,21 @@ uses_hard_regs_p (rtx x, HARD_REG_SET set)
      return false;
    code = GET_CODE (x);
    mode = GET_MODE (x);
+
    if (code == SUBREG)
      {
+      /* For all SUBREGs we want to check whether the full multi-register
+     overlaps the set.  For normal SUBREGs this means 'get_hard_regno' of
+     the inner register, for paradoxical SUBREGs this means the
+     'get_hard_regno' of the full SUBREG and for complete SUBREGs 
either is
+     fine.  Use the wider mode for all cases.  */
+      rtx subreg = SUBREG_REG (x);
        mode = wider_subreg_mode (x);
-      x = SUBREG_REG (x);
-      code = GET_CODE (x);
+      if (mode == GET_MODE (subreg))
+    {
+      x = subreg;
+      code = GET_CODE (x);
+    }
      }

    if (REG_P (x))

In your case, x will be SUBREG and be processed recursively only as a 
register of subreg.

I think you need to change the last line on

   if (REG_P (x) || code == SUBREG)

then the subreg will processed by get_hard_regno as subreg.

diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 0ef13439b5055dd2c5d5049d7f62f6b3b1ddfe2a..3af41b6eed2dc113c3158e1bde1c65a896d8feb5 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -1761,14 +1761,24 @@ uses_hard_regs_p (rtx x, HARD_REG_SET set)
     return false;
   code = GET_CODE (x);
   mode = GET_MODE (x);
+
   if (code == SUBREG)
     {
+      /* For all SUBREGs we want to check whether the full multi-register
+	 overlaps the set.  For normal SUBREGs this means 'get_hard_regno' of
+	 the inner register, for paradoxical SUBREGs this means the
+	 'get_hard_regno' of the full SUBREG and for complete SUBREGs either is
+	 fine.  Use the wider mode for all cases.  */
+      rtx subreg = SUBREG_REG (x);
       mode = wider_subreg_mode (x);
-      x = SUBREG_REG (x);
-      code = GET_CODE (x);
+      if (mode == GET_MODE (subreg))
+	{
+	  x = subreg;
+	  code = GET_CODE (x);
+	}
     }
 
-  if (REG_P (x))
+  if (REG_P (x) || SUBREG_P (x))
     {
       x_hard_regno = get_hard_regno (x, true);
       return (x_hard_regno >= 0
diff --git a/gcc/testsuite/gcc.target/arm/pr86487.c b/gcc/testsuite/gcc.target/arm/pr86487.c
new file mode 100644
index 0000000000000000000000000000000000000000..1c1db7852d91a82a1d2b6eaa4f3d4c6dbef107f5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr86487.c
@@ -0,0 +1,10 @@
+/* { dg-skip-if "" { *-*-* } { "-march=armv[0-6]*" "-mthumb" } { "" } } */
+/* { dg-require-effective-target arm_neon_hw } */
+/* { dg-options "-O1 -mbig-endian" } */
+/* { dg-add-options arm_neon } */
+int a, b, c, d;
+long long fn1(long long p2) { return p2 == 0 ? -1 : -1 % p2; }
+void fn2(long long p1, short p2, long p3) {
+  b = fn1((d || 6) & a);
+  c = b | p3;
+}