* [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization
@ 2017-01-09 9:51 Thomas Preudhomme
2017-01-13 18:19 ` Thomas Preudhomme
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Preudhomme @ 2017-01-09 9:51 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]
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
[-- Attachment #2: fix_pr78617_gcc5.patch --]
[-- Type: text/x-patch, Size: 1908 bytes --]
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, ®_obstack);
bitmap_initialize (&active_cands, ®_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;
+}
[-- Attachment #3: fix_pr78617_gcc6.patch --]
[-- Type: text/x-patch, Size: 1908 bytes --]
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, ®_obstack);
bitmap_initialize (&active_cands, ®_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;
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization
2017-01-09 9:51 [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization Thomas Preudhomme
@ 2017-01-13 18:19 ` Thomas Preudhomme
2017-01-16 19:26 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Preudhomme @ 2017-01-13 18:19 UTC (permalink / raw)
To: gcc-patches, Vladimir Makarov, Richard Biener, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]
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
[-- Attachment #2: fix_pr78617_gcc5.patch --]
[-- Type: text/x-patch, Size: 1908 bytes --]
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, ®_obstack);
bitmap_initialize (&active_cands, ®_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;
+}
[-- Attachment #3: fix_pr78617_gcc6.patch --]
[-- Type: text/x-patch, Size: 1908 bytes --]
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, ®_obstack);
bitmap_initialize (&active_cands, ®_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;
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization
2017-01-13 18:19 ` Thomas Preudhomme
@ 2017-01-16 19:26 ` Jeff Law
2017-01-16 21:12 ` Vladimir Makarov
2017-01-17 16:27 ` Bernd Schmidt
0 siblings, 2 replies; 6+ messages in thread
From: Jeff Law @ 2017-01-16 19:26 UTC (permalink / raw)
To: Thomas Preudhomme, gcc-patches, Vladimir Makarov, Richard Biener,
Jakub Jelinek
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization
2017-01-16 19:26 ` Jeff Law
@ 2017-01-16 21:12 ` Vladimir Makarov
2017-01-17 16:27 ` Bernd Schmidt
1 sibling, 0 replies; 6+ messages in thread
From: Vladimir Makarov @ 2017-01-16 21:12 UTC (permalink / raw)
To: Jeff Law, Thomas Preudhomme, gcc-patches, Richard Biener, Jakub Jelinek
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization
2017-01-16 19:26 ` Jeff Law
2017-01-16 21:12 ` Vladimir Makarov
@ 2017-01-17 16:27 ` Bernd Schmidt
2017-01-17 16:39 ` Jakub Jelinek
1 sibling, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2017-01-17 16:27 UTC (permalink / raw)
To: Jeff Law, Thomas Preudhomme, gcc-patches, Vladimir Makarov,
Richard Biener, Jakub Jelinek
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization
2017-01-17 16:27 ` Bernd Schmidt
@ 2017-01-17 16:39 ` Jakub Jelinek
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2017-01-17 16:39 UTC (permalink / raw)
To: Bernd Schmidt
Cc: Jeff Law, Thomas Preudhomme, gcc-patches, Vladimir Makarov,
Richard Biener
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-17 16:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-09 9:51 [PATCH, GCC/LRA, gcc-5/6-branch] Fix PR78617: Fix conflict detection in rematerialization Thomas Preudhomme
2017-01-13 18:19 ` Thomas Preudhomme
2017-01-16 19:26 ` Jeff Law
2017-01-16 21:12 ` Vladimir Makarov
2017-01-17 16:27 ` Bernd Schmidt
2017-01-17 16:39 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).