Re: Re: [RFC/PATCH v2][PR89245] Check REG_CALL_DECL note during the tail-merging
On 01.10.2019. 21:35, Jeff Law wrote: > On 9/6/19 4:23 AM, Dragan Mladjenovic wrote: >> On 24.07.2019. 20:57, Jeff Law wrote: >>> On 7/17/19 2:29 AM, Dragan Mladjenovic wrote: On 09.07.2019. 23:21, Jeff Law wrote: > On 7/9/19 2:06 PM, Dragan Mladjenovic wrote: >> This patch prevents merging of CALL instructions that that have different >> REG_CALL_DECL notes attached to them. >> >> On most architectures this is not an important distinction. Usually >> instruction patterns >> for calls to different functions reference different SYMBOL_REF-s, so >> they won't match. >> On MIPS PIC calls get split into an got_load/*call_internal pair where >> the latter represents >> indirect register call w/o SYMBOL_REF attached (until machine_reorg >> pass). The bugzilla issue >> had such two internal_call-s merged despite the fact that they had >> different register usage >> information assigned by ipa-ra. >> >> As per comment form Richard Sandiford, this version compares reg usage >> for both call >> instruction instead of shallow comparing the notes. Tests updated >> accordingly. >> >> gcc/ChangeLog: >> >> 2019-07-09 Dragan Mladjenovic >> >> * cfgcleanup.c (old_insns_match_p): Check if used hard regs set is equal >> for both call instructions. >> >> gcc/testsuite/ChangeLog: >> >> 2019-07-09 Dragan Mladjenovic >> >> * gcc.target/mips/cfgcleanup-jalr1.c: New test. >> * gcc.target/mips/cfgcleanup-jalr2.c: New test. >> * gcc.target/mips/cfgcleanup-jalr3.c: New test. > THanks. I've installed this on the trunk. > > jeff Thanks. Can this be back-ported to active branches also. This issue seems to be there > since gcc6 if not gcc5. >>> I've asked Matthew to handle the backport. I'm going to be on PTO the >>> next couple weeks. >>> >>> jeff >>> >> >> Hi, >> >> Sorry, I forgot to check up on this patch. Is it still ok for me to try >> to backport it to gcc 9 and gcc 8 branches? > Yes, this would be fine to backport to gcc-8 and gcc-9 branches. I'd > expect it to be pretty easy as I don't think old_insns_match_p has > changed much. Thanks and sorry for the delay. Backported as r277625 and r277626 to gcc-9 and gcc-8 branch respectively. Best regards, Dragan
Re: [EXTERNAL]Re: [RFC/PATCH v2][PR89245] Check REG_CALL_DECL note during the tail-merging
On 9/6/19 4:23 AM, Dragan Mladjenovic wrote: > On 24.07.2019. 20:57, Jeff Law wrote: >> On 7/17/19 2:29 AM, Dragan Mladjenovic wrote: >>> >>> >>> On 09.07.2019. 23:21, Jeff Law wrote: On 7/9/19 2:06 PM, Dragan Mladjenovic wrote: > This patch prevents merging of CALL instructions that that have different > REG_CALL_DECL notes attached to them. > > On most architectures this is not an important distinction. Usually > instruction patterns > for calls to different functions reference different SYMBOL_REF-s, so > they won't match. > On MIPS PIC calls get split into an got_load/*call_internal pair where > the latter represents > indirect register call w/o SYMBOL_REF attached (until machine_reorg > pass). The bugzilla issue > had such two internal_call-s merged despite the fact that they had > different register usage > information assigned by ipa-ra. > > As per comment form Richard Sandiford, this version compares reg usage > for both call > instruction instead of shallow comparing the notes. Tests updated > accordingly. > > gcc/ChangeLog: > > 2019-07-09 Dragan Mladjenovic > > * cfgcleanup.c (old_insns_match_p): Check if used hard regs set is equal > for both call instructions. > > gcc/testsuite/ChangeLog: > > 2019-07-09 Dragan Mladjenovic > > * gcc.target/mips/cfgcleanup-jalr1.c: New test. > * gcc.target/mips/cfgcleanup-jalr2.c: New test. > * gcc.target/mips/cfgcleanup-jalr3.c: New test. THanks. I've installed this on the trunk. jeff >>> Thanks. Can this be back-ported to active branches also. This issue >>> seems to be there > since gcc6 if not gcc5. >> I've asked Matthew to handle the backport. I'm going to be on PTO the >> next couple weeks. >> >> jeff >> > > Hi, > > Sorry, I forgot to check up on this patch. Is it still ok for me to try > to backport it to gcc 9 and gcc 8 branches? Yes, this would be fine to backport to gcc-8 and gcc-9 branches. I'd expect it to be pretty easy as I don't think old_insns_match_p has changed much. Jeff
Re: [EXTERNAL]Re: [RFC/PATCH v2][PR89245] Check REG_CALL_DECL note during the tail-merging
On 24.07.2019. 20:57, Jeff Law wrote: > On 7/17/19 2:29 AM, Dragan Mladjenovic wrote: >> >> >> On 09.07.2019. 23:21, Jeff Law wrote: >>> On 7/9/19 2:06 PM, Dragan Mladjenovic wrote: This patch prevents merging of CALL instructions that that have different REG_CALL_DECL notes attached to them. On most architectures this is not an important distinction. Usually instruction patterns for calls to different functions reference different SYMBOL_REF-s, so they won't match. On MIPS PIC calls get split into an got_load/*call_internal pair where the latter represents indirect register call w/o SYMBOL_REF attached (until machine_reorg pass). The bugzilla issue had such two internal_call-s merged despite the fact that they had different register usage information assigned by ipa-ra. As per comment form Richard Sandiford, this version compares reg usage for both call instruction instead of shallow comparing the notes. Tests updated accordingly. gcc/ChangeLog: 2019-07-09 Dragan Mladjenovic * cfgcleanup.c (old_insns_match_p): Check if used hard regs set is equal for both call instructions. gcc/testsuite/ChangeLog: 2019-07-09 Dragan Mladjenovic * gcc.target/mips/cfgcleanup-jalr1.c: New test. * gcc.target/mips/cfgcleanup-jalr2.c: New test. * gcc.target/mips/cfgcleanup-jalr3.c: New test. >>> THanks. I've installed this on the trunk. >>> >>> jeff >> Thanks. Can this be back-ported to active branches also. This issue >> seems to be there > since gcc6 if not gcc5. > I've asked Matthew to handle the backport. I'm going to be on PTO the > next couple weeks. > > jeff > Hi, Sorry, I forgot to check up on this patch. Is it still ok for me to try to backport it to gcc 9 and gcc 8 branches? Best regards, Dragan
Re: [RFC/PATCH v2][PR89245] Check REG_CALL_DECL note during the tail-merging
On 7/17/19 2:29 AM, Dragan Mladjenovic wrote: > > > On 09.07.2019. 23:21, Jeff Law wrote: >> On 7/9/19 2:06 PM, Dragan Mladjenovic wrote: >>> This patch prevents merging of CALL instructions that that have different >>> REG_CALL_DECL notes attached to them. >>> >>> On most architectures this is not an important distinction. Usually >>> instruction patterns >>> for calls to different functions reference different SYMBOL_REF-s, so they >>> won't match. >>> On MIPS PIC calls get split into an got_load/*call_internal pair where the >>> latter represents >>> indirect register call w/o SYMBOL_REF attached (until machine_reorg pass). >>> The bugzilla issue >>> had such two internal_call-s merged despite the fact that they had >>> different register usage >>> information assigned by ipa-ra. >>> >>> As per comment form Richard Sandiford, this version compares reg usage for >>> both call >>> instruction instead of shallow comparing the notes. Tests updated >>> accordingly. >>> >>> gcc/ChangeLog: >>> >>> 2019-07-09 Dragan Mladjenovic >>> >>> * cfgcleanup.c (old_insns_match_p): Check if used hard regs set is equal >>> for both call instructions. >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2019-07-09 Dragan Mladjenovic >>> >>> * gcc.target/mips/cfgcleanup-jalr1.c: New test. >>> * gcc.target/mips/cfgcleanup-jalr2.c: New test. >>> * gcc.target/mips/cfgcleanup-jalr3.c: New test. >> THanks. I've installed this on the trunk. >> >> jeff > Thanks. Can this be back-ported to active branches also. This issue > seems to be there > since gcc6 if not gcc5. I've asked Matthew to handle the backport. I'm going to be on PTO the next couple weeks. jeff
Re: [RFC/PATCH v2][PR89245] Check REG_CALL_DECL note during the tail-merging
On 09.07.2019. 23:21, Jeff Law wrote: > On 7/9/19 2:06 PM, Dragan Mladjenovic wrote: >> This patch prevents merging of CALL instructions that that have different >> REG_CALL_DECL notes attached to them. >> >> On most architectures this is not an important distinction. Usually >> instruction patterns >> for calls to different functions reference different SYMBOL_REF-s, so they >> won't match. >> On MIPS PIC calls get split into an got_load/*call_internal pair where the >> latter represents >> indirect register call w/o SYMBOL_REF attached (until machine_reorg pass). >> The bugzilla issue >> had such two internal_call-s merged despite the fact that they had different >> register usage >> information assigned by ipa-ra. >> >> As per comment form Richard Sandiford, this version compares reg usage for >> both call >> instruction instead of shallow comparing the notes. Tests updated >> accordingly. >> >> gcc/ChangeLog: >> >> 2019-07-09 Dragan Mladjenovic >> >> * cfgcleanup.c (old_insns_match_p): Check if used hard regs set is equal >> for both call instructions. >> >> gcc/testsuite/ChangeLog: >> >> 2019-07-09 Dragan Mladjenovic >> >> * gcc.target/mips/cfgcleanup-jalr1.c: New test. >> * gcc.target/mips/cfgcleanup-jalr2.c: New test. >> * gcc.target/mips/cfgcleanup-jalr3.c: New test. > THanks. I've installed this on the trunk. > > jeff Thanks. Can this be back-ported to active branches also. This issue seems to be there since gcc6 if not gcc5. Thanks in advance, Dragan
Re: [RFC/PATCH v2][PR89245] Check REG_CALL_DECL note during the tail-merging
On 7/9/19 2:06 PM, Dragan Mladjenovic wrote: > This patch prevents merging of CALL instructions that that have different > REG_CALL_DECL notes attached to them. > > On most architectures this is not an important distinction. Usually > instruction patterns > for calls to different functions reference different SYMBOL_REF-s, so they > won't match. > On MIPS PIC calls get split into an got_load/*call_internal pair where the > latter represents > indirect register call w/o SYMBOL_REF attached (until machine_reorg pass). > The bugzilla issue > had such two internal_call-s merged despite the fact that they had different > register usage > information assigned by ipa-ra. > > As per comment form Richard Sandiford, this version compares reg usage for > both call > instruction instead of shallow comparing the notes. Tests updated accordingly. > > gcc/ChangeLog: > > 2019-07-09 Dragan Mladjenovic > > * cfgcleanup.c (old_insns_match_p): Check if used hard regs set is equal > for both call instructions. > > gcc/testsuite/ChangeLog: > > 2019-07-09 Dragan Mladjenovic > > * gcc.target/mips/cfgcleanup-jalr1.c: New test. > * gcc.target/mips/cfgcleanup-jalr2.c: New test. > * gcc.target/mips/cfgcleanup-jalr3.c: New test. THanks. I've installed this on the trunk. jeff
[RFC/PATCH v2][PR89245] Check REG_CALL_DECL note during the tail-merging
This patch prevents merging of CALL instructions that that have different
REG_CALL_DECL notes attached to them.
On most architectures this is not an important distinction. Usually instruction
patterns
for calls to different functions reference different SYMBOL_REF-s, so they
won't match.
On MIPS PIC calls get split into an got_load/*call_internal pair where the
latter represents
indirect register call w/o SYMBOL_REF attached (until machine_reorg pass). The
bugzilla issue
had such two internal_call-s merged despite the fact that they had different
register usage
information assigned by ipa-ra.
As per comment form Richard Sandiford, this version compares reg usage for both
call
instruction instead of shallow comparing the notes. Tests updated accordingly.
gcc/ChangeLog:
2019-07-09 Dragan Mladjenovic
* cfgcleanup.c (old_insns_match_p): Check if used hard regs set is equal
for both call instructions.
gcc/testsuite/ChangeLog:
2019-07-09 Dragan Mladjenovic
* gcc.target/mips/cfgcleanup-jalr1.c: New test.
* gcc.target/mips/cfgcleanup-jalr2.c: New test.
* gcc.target/mips/cfgcleanup-jalr3.c: New test.
---
gcc/cfgcleanup.c | 9 +
gcc/testsuite/gcc.target/mips/cfgcleanup-jalr1.c | 19 +++
gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c | 23 +++
gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c | 23 +++
4 files changed, 74 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/mips/cfgcleanup-jalr1.c
create mode 100644 gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c
create mode 100644 gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 992912c..fca3a08 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -53,6 +53,7 @@ along with GCC; see the file COPYING3. If not see
#include "dce.h"
#include "dbgcnt.h"
#include "rtl-iter.h"
+#include "regs.h"
#define FORWARDER_BLOCK_P(BB) ((BB)->flags & BB_FORWARDER_BLOCK)
@@ -1224,6 +1225,14 @@ old_insns_match_p (int mode ATTRIBUTE_UNUSED, rtx_insn
*i1, rtx_insn *i2)
}
}
}
+
+ HARD_REG_SET i1_used, i2_used;
+
+ get_call_reg_set_usage (i1, _used, call_used_reg_set);
+ get_call_reg_set_usage (i2, _used, call_used_reg_set);
+
+ if (!hard_reg_set_equal_p (i1_used, i2_used))
+return dir_none;
}
/* If both i1 and i2 are frame related, verify all the CFA notes
diff --git a/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr1.c
b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr1.c
new file mode 100644
index 000..24c1826
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr1.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-mabicalls -fpic -mno-mips16 -mno-micromips" } */
+/* { dg-skip-if "needs codesize optimization" { *-*-* } { "-O0" "-O1" "-O2"
"-O3" } { "" } } */
+
+extern void foo (void*);
+
+extern void bar (void*);
+
+void
+test (void* p)
+{
+ if (!p)
+ foo(p);
+ else
+ bar(p);
+}
+
+/* { dg-final { scan-assembler-not "\\\.reloc\t1f,R_MIPS_JALR,foo" } } */
+/* { dg-final { scan-assembler-not "\\\.reloc\t1f,R_MIPS_JALR,bar" } } */
diff --git a/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c
b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c
new file mode 100644
index 000..9fd75c9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-mabicalls -fpic -mno-mips16 -mno-micromips" } */
+/* { dg-additional-options "-fno-inline -fipa-ra -mcompact-branches=never" } */
+/* { dg-skip-if "needs codesize optimization" { *-*-* } { "-O0" "-O1" "-O2"
"-O3" } { "" } } */
+
+static int foo (void* p) { __asm__ (""::"r"(p):"$t0"); return 0; }
+
+static int bar (void* p) { return 1; }
+
+int
+test (void* p)
+{
+ int res = !p ? foo(p) : bar(p);
+
+ register int tmp __asm__("$t0") = -1;
+ __asm__ (""::"r"(tmp));
+
+ return res;
+}
+
+/* { dg-final { scan-assembler "\\\.reloc\t1f,R_MIPS_JALR,foo" } } */
+/* { dg-final { scan-assembler "\\\.reloc\t1f,R_MIPS_JALR,bar" } } */
+/* { dg-final { scan-assembler-not "\\.set\tnomacro\n\tjalr\t\\\$25" } } */
diff --git a/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c
b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c
new file mode 100644
index 000..580c6ec
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-mabicalls -fpic -mno-mips16 -mno-micromips" } */
+/* { dg-additional-options "-fno-inline -fipa-ra -mcompact-branches=never" } */
+/* { dg-skip-if "needs codesize optimization" { *-*-* } { "-O0" "-O1" "-O2"
"-O3" } { "" } } */
+
+static int foo (void* p) { return 0; }
+
+static int bar (void* p) { return 1; }
+
+int
+test (void* p)
+{
+ int res = !p ? foo(p) : bar(p);
+
+ register int tmp __asm__("$t0") = -1;
+ __asm__ (""::"r"(tmp));
+
+ return res;
