diff mbox

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

Message ID 913bab85-c591-22a6-37b3-31e1f57ae77b@foss.arm.com
State New
Headers show

Commit Message

Thomas Preudhomme Jan. 13, 2017, 6:19 p.m. UTC 
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.preudhomme@arm.com>

>

>         Backport from mainline

>         2016-12-07 Thomas Preud'homme <thomas.preudhomme@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.preudhomme@arm.com>

>

>         Backport from mainline

>         2016-12-07 Thomas Preud'homme <thomas.preudhomme@arm.com>

>

>         PR rtl-optimization/78617

>         * gcc.c-torture/execute/pr78617.c: New test.

>

>

> Best regards,

>

> Thomas

Comments

Jeff Law Jan. 16, 2017, 7:26 p.m. UTC | #1 
On 01/13/2017 11:19 AM, Thomas Preudhomme wrote:
> Ping? I'm not sure if an ok from Valdimir is enough or if I also need RM

> approval.

Vlad's approval is all you need.


jeff
Vladimir Makarov Jan. 16, 2017, 9:12 p.m. UTC | #2 
On 01/16/2017 02:26 PM, Jeff Law wrote:
> On 01/13/2017 11:19 AM, Thomas Preudhomme wrote:

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

>> approval.

> Vlad's approval is all you need.

>

Thomas, the patch is ok for backporting.  It is pretty safe.
Bernd Schmidt Jan. 17, 2017, 4:22 p.m. UTC | #3 
On 01/16/2017 08:26 PM, Jeff Law wrote:
> On 01/13/2017 11:19 AM, Thomas Preudhomme wrote:

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

>> approval.

> Vlad's approval is all you need.


Is that a general rule? I'm never too certain on that.


Bernd
Jakub Jelinek Jan. 17, 2017, 4:27 p.m. UTC | #4 
On Tue, Jan 17, 2017 at 05:22:34PM +0100, Bernd Schmidt wrote:
> On 01/16/2017 08:26 PM, Jeff Law wrote:

> > On 01/13/2017 11:19 AM, Thomas Preudhomme wrote:

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

> > > approval.

> > Vlad's approval is all you need.

> 

> Is that a general rule? I'm never too certain on that.


Unless the branch is frozen for release (at which point all commits need RM
approval) or closed, maintainers/reviewers can approve backports in the
areas they are maintainers or reviewers for.
Of course good judgement should be used on what should be backported and
what should not.

	Jakub
diff mbox

Patch

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;
+}