Re: [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization

[Thomas Preudhomme]

Ping? I'm not sure if an ok from Valdimir is enough or if I also need RM 
approval.

Best regards,

Thomas

On 09/01/17 09:51, Thomas Preudhomme wrote:

Hi,

Is it ok to backport the fix for PR78617 (incorrect conflict detection in
rematerialization) to GCC 5 and GCC 6? The patch applies cleanly and the
testsuite showed no regression when performed with the following configurations:

- an arm-none-eabi GCC cross-compiler targeting Cortex-M0 and Cortex-M3
- a bootstrapped arm-linux-gnueabihf GCC native compiler
- a bootstrapped x86_64-linux-gnu GCC native compiler

ChangeLog entry is as follows:

*** gcc/ChangeLog ***

2017-01-03 Thomas Preud'homme <thomas.preudho...@arm.com>

        Backport from mainline
        2016-12-07 Thomas Preud'homme <thomas.preudho...@arm.com>

        PR rtl-optimization/78617
        * lra-remat.c (do_remat): Initialize live_hard_regs from live in
        registers, also setting hard registers mapped to pseudo registers.


*** gcc/testsuite/ChangeLog ***


2017-01-03 Thomas Preud'homme <thomas.preudho...@arm.com>

        Backport from mainline
        2016-12-07 Thomas Preud'homme <thomas.preudho...@arm.com>

        PR rtl-optimization/78617
        * gcc.c-torture/execute/pr78617.c: New test.


Best regards,

Thomas

diff --git a/gcc/lra-remat.c b/gcc/lra-remat.c
index 5e5d62c50b011fe53c5652a4406d711feb448885..17da91b7f2144b2eaf48ce13f547239013c6e7c3 100644
--- a/gcc/lra-remat.c
+++ b/gcc/lra-remat.c
@@ -1124,6 +1124,7 @@ update_scratch_ops (rtx_insn *remat_insn)
 static bool
 do_remat (void)
 {
+  unsigned regno;
   rtx_insn *insn;
   basic_block bb;
   bitmap_head avail_cands;
@@ -1131,12 +1132,21 @@ do_remat (void)
   bool changed_p = false;
   /* Living hard regs and hard registers of living pseudos.  */
   HARD_REG_SET live_hard_regs;
+  bitmap_iterator bi;
 
   bitmap_initialize (&avail_cands, &reg_obstack);
   bitmap_initialize (&active_cands, &reg_obstack);
   FOR_EACH_BB_FN (bb, cfun)
     {
-      REG_SET_TO_HARD_REG_SET (live_hard_regs, df_get_live_out (bb));
+      CLEAR_HARD_REG_SET (live_hard_regs);
+      EXECUTE_IF_SET_IN_BITMAP (df_get_live_in (bb), 0, regno, bi)
+	{
+	  int hard_regno = regno < FIRST_PSEUDO_REGISTER
+			   ? regno
+			   : reg_renumber[regno];
+	  if (hard_regno >= 0)
+	    SET_HARD_REG_BIT (live_hard_regs, hard_regno);
+	}
       bitmap_and (&avail_cands, &get_remat_bb_data (bb)->avin_cands,
 		  &get_remat_bb_data (bb)->livein_cands);
       /* Activating insns are always in the same block as their corresponding
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr78617.c b/gcc/testsuite/gcc.c-torture/execute/pr78617.c
new file mode 100644
index 0000000000000000000000000000000000000000..89c4f6dea8cb507b963f91debb94cbe16eb1db90
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr78617.c
@@ -0,0 +1,25 @@
+int a = 0;
+int d = 1;
+int f = 1;
+
+int fn1() {
+  return a || 1 >> a;
+}
+
+int fn2(int p1, int p2) {
+  return p2 >= 2 ? p1 : p1 >> 1;
+}
+
+int fn3(int p1) {
+  return d ^ p1;
+}
+
+int fn4(int p1, int p2) {
+  return fn3(!d > fn2((f = fn1() - 1000) || p2, p1));
+}
+
+int main() {
+  if (fn4(0, 0) != 1)
+    __builtin_abort ();
+  return 0;
+}

diff --git a/gcc/lra-remat.c b/gcc/lra-remat.c
index 187ee3e7752d1ebe15ba8e8014620c0a94e11424..79504d4eb1a052d906a69178d847e6e618b468ec 100644
--- a/gcc/lra-remat.c
+++ b/gcc/lra-remat.c
@@ -1116,6 +1116,7 @@ update_scratch_ops (rtx_insn *remat_insn)
 static bool
 do_remat (void)
 {
+  unsigned regno;
   rtx_insn *insn;
   basic_block bb;
   bitmap_head avail_cands;
@@ -1123,12 +1124,21 @@ do_remat (void)
   bool changed_p = false;
   /* Living hard regs and hard registers of living pseudos.  */
   HARD_REG_SET live_hard_regs;
+  bitmap_iterator bi;
 
   bitmap_initialize (&avail_cands, &reg_obstack);
   bitmap_initialize (&active_cands, &reg_obstack);
   FOR_EACH_BB_FN (bb, cfun)
     {
-      REG_SET_TO_HARD_REG_SET (live_hard_regs, df_get_live_out (bb));
+      CLEAR_HARD_REG_SET (live_hard_regs);
+      EXECUTE_IF_SET_IN_BITMAP (df_get_live_in (bb), 0, regno, bi)
+	{
+	  int hard_regno = regno < FIRST_PSEUDO_REGISTER
+			   ? regno
+			   : reg_renumber[regno];
+	  if (hard_regno >= 0)
+	    SET_HARD_REG_BIT (live_hard_regs, hard_regno);
+	}
       bitmap_and (&avail_cands, &get_remat_bb_data (bb)->avin_cands,
 		  &get_remat_bb_data (bb)->livein_cands);
       /* Activating insns are always in the same block as their corresponding
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr78617.c b/gcc/testsuite/gcc.c-torture/execute/pr78617.c
new file mode 100644
index 0000000000000000000000000000000000000000..89c4f6dea8cb507b963f91debb94cbe16eb1db90
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr78617.c
@@ -0,0 +1,25 @@
+int a = 0;
+int d = 1;
+int f = 1;
+
+int fn1() {
+  return a || 1 >> a;
+}
+
+int fn2(int p1, int p2) {
+  return p2 >= 2 ? p1 : p1 >> 1;
+}
+
+int fn3(int p1) {
+  return d ^ p1;
+}
+
+int fn4(int p1, int p2) {
+  return fn3(!d > fn2((f = fn1() - 1000) || p2, p1));
+}
+
+int main() {
+  if (fn4(0, 0) != 1)
+    __builtin_abort ();
+  return 0;
+}