RE: Fix for "FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_x_tst.o compile, (internal compiler error)"

["David Sherwood" <david.sherwood@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 2659 bytes --]

Hi Vladimir,

Sorry this took so long. I have tidied up the patch as you suggested and fixed some
style issues. Hope this looks better now.

Thanks!
David.

2014-09-26  David Sherwood  <david.sherwood@arm.com>

        * ira-int.h (ira_allocno): Add "wmode" field.
        * ira-build.c (create_insn_allocnos): Add new "parent" function
        parameter.
        * ira-conflicts.c (ira_build_conflicts): Add conflicts for registers
        that cannot be accessed in wmode.


-----Original Message-----
From: David Sherwood [mailto:david.sherwood@arm.com] 
Sent: 08 September 2014 12:48
To: 'gcc-patches@gcc.gnu.org'
Cc: 'vmakarov@redhat.com'
Subject: RE: Fix for "FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_x_tst.o compile, (internal
compiler error)"

Hi Vladimir,

Sorry, I forgot to CC you on this as it's your code. It's my first attempt at
submitting patches to gcc so I'm still learning as I go!

Kind Regards,
David Sherwood.

-----Original Message-----
From: David Sherwood [mailto:david.sherwood@arm.com] 
Sent: 05 September 2014 15:52
To: 'gcc-patches@gcc.gnu.org'
Subject: Fix for "FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_x_tst.o compile, (internal
compiler error)"

Hi,

I have a potential fix for a gcc testsuite failure for aarch64 in big endian, i.e.

FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_x_tst.o compile, (internal compiler error)
FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_y_tst.o compile, (internal compiler error)

It is caused by the inappropriate choice of hard registers for paradoxical sub registers in
big endian mode, for example if register 0 is chosen for a paradoxical TI subreg on big
endian then we may end up attempting to reference register -1. Similarly, on little endian
we could end up going beyond the upper bounds of the register file too.

My fix involves adding particular constraints in IRA on the choice of register once paradoxical 
sub registers are encountered. However, Richard Sandiford also proposed an alternative
solution that involves not constraining registers in IRA, but rather making use of cost analysis
instead and letting LRA do the work. Not sure what your preference is ....

Fix was tested on aarch64 on little and big endian with no regressions.

Regards,
David Sherwood.

2014-08-26  David Sherwood  <david.sherwood@arm.com>

        * ira-int.h (ira_allocno): Add "wmode" field.
        * ira-build.c (create_insn_allocnos): Add new "parent" function
        parameter.
        * ira-conflicts.c (ira_build_conflicts): Add conflicts for registers
        that cannot be accessed in wmode.

[-- Attachment #2: rb2369.patch --]
[-- Type: application/octet-stream, Size: 6805 bytes --]

diff --git a/gcc/ira-build.c b/gcc/ira-build.c
index 3f2ab17..ddea1f6 100644
--- a/gcc/ira-build.c
+++ b/gcc/ira-build.c
@@ -524,6 +524,7 @@ ira_create_allocno (int regno, bool cap_p,
   ALLOCNO_BAD_SPILL_P (a) = false;
   ALLOCNO_ASSIGNED_P (a) = false;
   ALLOCNO_MODE (a) = (regno < 0 ? VOIDmode : PSEUDO_REGNO_MODE (regno));
+  ALLOCNO_WMODE (a) = ALLOCNO_MODE (a);
   ALLOCNO_PREFS (a) = NULL;
   ALLOCNO_COPIES (a) = NULL;
   ALLOCNO_HARD_REG_COSTS (a) = NULL;
@@ -893,6 +894,7 @@ create_cap_allocno (ira_allocno_t a)
   parent = ALLOCNO_LOOP_TREE_NODE (a)->parent;
   cap = ira_create_allocno (ALLOCNO_REGNO (a), true, parent);
   ALLOCNO_MODE (cap) = ALLOCNO_MODE (a);
+  ALLOCNO_WMODE (cap) = ALLOCNO_WMODE (a);
   aclass = ALLOCNO_CLASS (a);
   ira_set_allocno_class (cap, aclass);
   ira_create_allocno_objects (cap);
@@ -1859,9 +1861,11 @@ static basic_block curr_bb;
 
 /* This recursive function creates allocnos corresponding to
    pseudo-registers containing in X.  True OUTPUT_P means that X is
-   a lvalue.  */
+   a lvalue.  The 'parent' parameter corresponds to the parent expression
+   of 'rtx'.
+ */
 static void
-create_insn_allocnos (rtx x, bool output_p)
+create_insn_allocnos (rtx x, rtx outer, bool output_p)
 {
   int i, j;
   const char *fmt;
@@ -1876,7 +1880,15 @@ create_insn_allocnos (rtx x, bool output_p)
 	  ira_allocno_t a;
 
 	  if ((a = ira_curr_regno_allocno_map[regno]) == NULL)
-	    a = ira_create_allocno (regno, false, ira_curr_loop_tree_node);
+            {
+              a = ira_create_allocno (regno, false, ira_curr_loop_tree_node);
+              if (outer != NULL && GET_CODE (outer) == SUBREG)
+                {
+                  enum machine_mode wmode = GET_MODE (outer);
+                  if (GET_MODE_SIZE (wmode) > GET_MODE_SIZE (ALLOCNO_WMODE (a)))
+                    ALLOCNO_WMODE (a) = wmode;
+                }
+            }
 
 	  ALLOCNO_NREFS (a)++;
 	  ALLOCNO_FREQ (a) += REG_FREQ_FROM_BB (curr_bb);
@@ -1887,25 +1899,25 @@ create_insn_allocnos (rtx x, bool output_p)
     }
   else if (code == SET)
     {
-      create_insn_allocnos (SET_DEST (x), true);
-      create_insn_allocnos (SET_SRC (x), false);
+      create_insn_allocnos (SET_DEST (x), NULL, true);
+      create_insn_allocnos (SET_SRC (x), NULL, false);
       return;
     }
   else if (code == CLOBBER)
     {
-      create_insn_allocnos (XEXP (x, 0), true);
+      create_insn_allocnos (XEXP (x, 0), NULL, true);
       return;
     }
   else if (code == MEM)
     {
-      create_insn_allocnos (XEXP (x, 0), false);
+      create_insn_allocnos (XEXP (x, 0), NULL, false);
       return;
     }
   else if (code == PRE_DEC || code == POST_DEC || code == PRE_INC ||
 	   code == POST_INC || code == POST_MODIFY || code == PRE_MODIFY)
     {
-      create_insn_allocnos (XEXP (x, 0), true);
-      create_insn_allocnos (XEXP (x, 0), false);
+      create_insn_allocnos (XEXP (x, 0), NULL, true);
+      create_insn_allocnos (XEXP (x, 0), NULL, false);
       return;
     }
 
@@ -1913,10 +1925,10 @@ create_insn_allocnos (rtx x, bool output_p)
   for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
     {
       if (fmt[i] == 'e')
-	create_insn_allocnos (XEXP (x, i), output_p);
+	create_insn_allocnos (XEXP (x, i), x, output_p);
       else if (fmt[i] == 'E')
 	for (j = 0; j < XVECLEN (x, i); j++)
-	  create_insn_allocnos (XVECEXP (x, i, j), output_p);
+	  create_insn_allocnos (XVECEXP (x, i, j), x, output_p);
     }
 }
 
@@ -1935,7 +1947,7 @@ create_bb_allocnos (ira_loop_tree_node_t bb_node)
   ira_assert (bb != NULL);
   FOR_BB_INSNS_REVERSE (bb, insn)
     if (NONDEBUG_INSN_P (insn))
-      create_insn_allocnos (PATTERN (insn), false);
+      create_insn_allocnos (PATTERN (insn), NULL, false);
   /* It might be a allocno living through from one subloop to
      another.  */
   EXECUTE_IF_SET_IN_REG_SET (df_get_live_in (bb), FIRST_PSEUDO_REGISTER, i, bi)
diff --git a/gcc/ira-conflicts.c b/gcc/ira-conflicts.c
index 011a865..ca71be2 100644
--- a/gcc/ira-conflicts.c
+++ b/gcc/ira-conflicts.c
@@ -774,6 +774,27 @@ ira_build_conflicts (void)
 				temp_hard_reg_set);
 	    }
 
+          /* Now we deal with paradoxical subreg cases where certain registers
+             cannot be accessed in the widest mode.  */
+          enum machine_mode outer_mode = ALLOCNO_WMODE (a);
+          enum machine_mode inner_mode = ALLOCNO_MODE (a);
+          if (GET_MODE_SIZE (outer_mode) > GET_MODE_SIZE (inner_mode))
+            {
+              enum reg_class aclass = ALLOCNO_CLASS (a);
+              for (int j = ira_class_hard_regs_num[aclass] - 1; j >= 0; --j)
+                {
+                   int inner_regno = ira_class_hard_regs[aclass][j];
+                   int outer_regno = simplify_subreg_regno (inner_regno,
+                                                            inner_mode, 0,
+                                                            outer_mode);
+                   if (outer_regno < 0 ||
+                       !in_hard_reg_set_p (reg_class_contents[aclass],
+                                           outer_mode, outer_regno))
+                     SET_HARD_REG_BIT (OBJECT_CONFLICT_HARD_REGS (obj),
+                                       inner_regno);
+                }
+            }
+
 	  if (ALLOCNO_CALLS_CROSSED_NUM (a) != 0)
 	    {
 	      int regno;
diff --git a/gcc/ira-int.h b/gcc/ira-int.h
index 1db0641..4035c6a 100644
--- a/gcc/ira-int.h
+++ b/gcc/ira-int.h
@@ -280,6 +280,9 @@ struct ira_allocno
   /* Mode of the allocno which is the mode of the corresponding
      pseudo-register.  */
   ENUM_BITFIELD (machine_mode) mode : 8;
+  /* Widest mode of the allocno which in at least one case could be
+     for paradoxical subregs where wmode > mode.  */
+  ENUM_BITFIELD (machine_mode) wmode : 8;
   /* Register class which should be used for allocation for given
      allocno.  NO_REGS means that we should use memory.  */
   ENUM_BITFIELD (reg_class) aclass : 16;
@@ -312,7 +315,7 @@ struct ira_allocno
      number (0, ...) - 2.  Value -1 is used for allocnos spilled by the
      reload (at this point pseudo-register has only one allocno) which
      did not get stack slot yet.  */
-  short int hard_regno;
+  int hard_regno : 16;
   /* Allocnos with the same regno are linked by the following member.
      Allocnos corresponding to inner loops are first in the list (it
      corresponds to depth-first traverse of the loops).  */
@@ -433,6 +436,7 @@ struct ira_allocno
 #define ALLOCNO_BAD_SPILL_P(A) ((A)->bad_spill_p)
 #define ALLOCNO_ASSIGNED_P(A) ((A)->assigned_p)
 #define ALLOCNO_MODE(A) ((A)->mode)
+#define ALLOCNO_WMODE(A) ((A)->wmode)
 #define ALLOCNO_PREFS(A) ((A)->allocno_prefs)
 #define ALLOCNO_COPIES(A) ((A)->allocno_copies)
 #define ALLOCNO_HARD_REG_COSTS(A) ((A)->hard_reg_costs)