Re: [PATCH] [RTL] Relax CSE check to set REG_EQUAL notes.
On 24/04/15 02:16, Jeff Law wrote: On 04/10/2015 03:14 AM, Alex Velenko wrote: On 09/03/15 17:40, Jeff Law wrote: On 03/09/15 03:53, Steven Bosscher wrote: On Wed, Mar 4, 2015 at 12:09 PM, Alex Velenko wrote: For example, in arm testcase pr43920-2.c, CSE previously decided not to put an obvious note on insn 9, as set value was the same as note value. At the same time, other insns set up as -1 were set up through a register and did get a note: ...which is the point of the REG_EQUAL notes. In insn 8 there is a REG_EQUAL note to show that the value of r111 is known. In insn 9 the known value is, well, known from SET_SRC so there is no need for a REG_EQUAL note. Adding REG_EQUAL notes in such cases is just wasteful. RIght. I'd rather look into why later passes aren't discovering whatever equivalences are important rather than adding the redundant notes. Regardless, I think this is a gcc-6 issue, so I'm not likely to look at it in the immediate future. jeff Hi Jeff, I reworked the patch to satisfy your preference. This patch enables cfgcleanup.c to use const int rtx as REG_EQUAL notes. For example, this benefits Jump2 to find extra optimisation opportunities. This patch fixes gcc.target/arm/pr43920-2.c for arm-none-eabi. Bootstraped on x86, run full regression run on arm-none-eabi and aarch64-none-elf. Is this patch ok? gcc/ 2015-03-17 Alex Velenko alex.vele...@arm.com * cfgcleanup.c (can_replace_by): Use const int rtx of single set as REG_EQUAL note. Now I finally see this in my queue. I recalled the discussion around whether or not to add the redundant notes, but hadn't had a chance to look at the updated patch. AFAICT, this is redundant with Shiva's patch, right? jeff Hi Jeff, Yes, you are correct, this patch is now redundant. Kind regards, Alex
Re: [PATCH] [RTL] Relax CSE check to set REG_EQUAL notes.
On 04/10/2015 03:14 AM, Alex Velenko wrote: On 09/03/15 17:40, Jeff Law wrote: On 03/09/15 03:53, Steven Bosscher wrote: On Wed, Mar 4, 2015 at 12:09 PM, Alex Velenko wrote: For example, in arm testcase pr43920-2.c, CSE previously decided not to put an obvious note on insn 9, as set value was the same as note value. At the same time, other insns set up as -1 were set up through a register and did get a note: ...which is the point of the REG_EQUAL notes. In insn 8 there is a REG_EQUAL note to show that the value of r111 is known. In insn 9 the known value is, well, known from SET_SRC so there is no need for a REG_EQUAL note. Adding REG_EQUAL notes in such cases is just wasteful. RIght. I'd rather look into why later passes aren't discovering whatever equivalences are important rather than adding the redundant notes. Regardless, I think this is a gcc-6 issue, so I'm not likely to look at it in the immediate future. jeff Hi Jeff, I reworked the patch to satisfy your preference. This patch enables cfgcleanup.c to use const int rtx as REG_EQUAL notes. For example, this benefits Jump2 to find extra optimisation opportunities. This patch fixes gcc.target/arm/pr43920-2.c for arm-none-eabi. Bootstraped on x86, run full regression run on arm-none-eabi and aarch64-none-elf. Is this patch ok? gcc/ 2015-03-17 Alex Velenko alex.vele...@arm.com * cfgcleanup.c (can_replace_by): Use const int rtx of single set as REG_EQUAL note. Now I finally see this in my queue. I recalled the discussion around whether or not to add the redundant notes, but hadn't had a chance to look at the updated patch. AFAICT, this is redundant with Shiva's patch, right? jeff
Re: [PATCH] [RTL] Relax CSE check to set REG_EQUAL notes.
On 03/09/15 03:53, Steven Bosscher wrote: On Wed, Mar 4, 2015 at 12:09 PM, Alex Velenko wrote: For example, in arm testcase pr43920-2.c, CSE previously decided not to put an obvious note on insn 9, as set value was the same as note value. At the same time, other insns set up as -1 were set up through a register and did get a note: ...which is the point of the REG_EQUAL notes. In insn 8 there is a REG_EQUAL note to show that the value of r111 is known. In insn 9 the known value is, well, known from SET_SRC so there is no need for a REG_EQUAL note. Adding REG_EQUAL notes in such cases is just wasteful. RIght. I'd rather look into why later passes aren't discovering whatever equivalences are important rather than adding the redundant notes. Regardless, I think this is a gcc-6 issue, so I'm not likely to look at it in the immediate future. jeff
Re: [PATCH] [RTL] Relax CSE check to set REG_EQUAL notes.
On Wed, Mar 4, 2015 at 12:09 PM, Alex Velenko wrote:
For example, in arm testcase pr43920-2.c, CSE previously decided not to put
an obvious note on insn 9, as set value was the same as note value.
At the same time, other insns set up as -1 were set up through a register
and did get a note:
...which is the point of the REG_EQUAL notes. In insn 8 there is a
REG_EQUAL note to show that the value of r111 is known. In insn 9 the
known value is, well, known from SET_SRC so there is no need for a
REG_EQUAL note. Adding REG_EQUAL notes in such cases is just wasteful.
(insn 9 53 34 8 (set (reg:SI 110 [ D.4934 ])
(const_int -1 [0x]))
/work/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:21 613
{*thumb2_movsi_vfp}
(nil))
(insn 8 45 50 6 (set (reg:SI 110 [ D.4934 ])
(reg/v:SI 111 [ startD.4917 ]))
/work/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:21 613
{*thumb2_movsi_vfp}
(expr_list:REG_EQUAL (const_int -1 [0x])
(nil)))
(insn 6 49 54 7 (set (reg:SI 110 [ D.4934 ])
(reg/v:SI 112 [ endD.4918 ]))
/work/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:21 613
{*thumb2_movsi_vfp}
(expr_list:REG_EQUAL (const_int -1 [0x])
(nil)))
Jump2 pass, optimizing common code, was looking at notes to reason about
register values and failing to recognize those insns to be equal.
I suppose you are talking about the head/tail merging code? Can you
please provide a test case for problem preferably filed in Bugzilla)?
Ciao!
Steven
Re: [PATCH] [RTL] Relax CSE check to set REG_EQUAL notes.
On Wed, Mar 04, 2015 at 11:09:14AM +, Alex Velenko wrote: I prefer adding notes in CSE instead of adding additional checks in Jump2 and, if any, other passes, as I think it is more uniform solution and allows single point fix. Downside is having more notes. The other downside is that every other RTL producer will have to add these notes as well, or passes that look at this info will have to look at the insns themselves anyway. Not a good trade-off in my opinion. Just add a simple helper function to do these checks (isn't there one already)? Segher
