Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn
On 02/15/2016 05:38 AM, Bernd Schmidt wrote: On 02/12/2016 08:43 AM, Jeff Law wrote: On 02/11/2016 06:28 PM, Bernd Schmidt wrote: PR rtl-optimization/69752 * ira.c (update_equiv_regs): When looking for more than a single SET, also take other side effects into account. OK for the trunk. Branches too? The problem obviously exists everywhere. Anywhere you deem appropriate. jeff
Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn
On 02/12/2016 08:43 AM, Jeff Law wrote: On 02/11/2016 06:28 PM, Bernd Schmidt wrote: PR rtl-optimization/69752 * ira.c (update_equiv_regs): When looking for more than a single SET, also take other side effects into account. OK for the trunk. Branches too? The problem obviously exists everywhere. Bernd
Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn
On 12/02/16 07:43, Jeff Law wrote:
On 02/11/2016 06:28 PM, Bernd Schmidt wrote:
This seems fairly straightforward:
(insn 213 455 216 6 (set (reg:SI 266)
(mem/u/c:SI (post_inc:SI (reg/f:SI 267)) [4 S4 A32])) 748
{*thumb1_movsi_insn}
(expr_list:REG_EQUAL (const_int -1044200508 [0xc1c2c3c4])
(expr_list:REG_INC (reg/f:SI 267)
(nil
We don't notice that the SET_SRC has a side effect, record the insn as
an equivalencing one, and later remove it because we replaced the reg
with the constant everywhere. Thus, the increment doesn't take place.
Fixed as follows. Bootstrapped and tested on x86_64-linux. Also compared
before/after dumps for the testcase with arm-elf. Ok?
Bernd
equiv-inc.diff
PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single SET,
also take other side effects into account.
Also note that the reporter says gcc-4.9 didn't have this problem, so
there's a reasonable chance this is a latent regression exposed by
codegen changes prior to IRA.
OK for the trunk.
Jeff
I tested it for the particular testcase it was failing for cortex-m0 and
it fixed it. Ill fire up a regression run next.
Cheers,
Andre
Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn
On 12/02/16 07:43, Jeff Law wrote:
On 02/11/2016 06:28 PM, Bernd Schmidt wrote:
This seems fairly straightforward:
(insn 213 455 216 6 (set (reg:SI 266)
(mem/u/c:SI (post_inc:SI (reg/f:SI 267)) [4 S4 A32])) 748
{*thumb1_movsi_insn}
(expr_list:REG_EQUAL (const_int -1044200508 [0xc1c2c3c4])
(expr_list:REG_INC (reg/f:SI 267)
(nil
We don't notice that the SET_SRC has a side effect, record the insn as
an equivalencing one, and later remove it because we replaced the reg
with the constant everywhere. Thus, the increment doesn't take place.
Fixed as follows. Bootstrapped and tested on x86_64-linux. Also compared
before/after dumps for the testcase with arm-elf. Ok?
Bernd
equiv-inc.diff
PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single
SET,
also take other side effects into account.
Will it be better that we don't remove the insn if it has side-effect
instead of don't record the equiv?
This can offer more equiv for later rtl optimization?
Also note that the reporter says gcc-4.9 didn't have this problem, so
there's a reasonable chance this is a latent regression exposed by
codegen changes prior to IRA.
OK for the trunk.
Jeff
Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn
On 12/02/16 13:33, Bernd Schmidt wrote: On 02/12/2016 02:18 PM, Jiong Wang wrote: PR rtl-optimization/69752 * ira.c (update_equiv_regs): When looking for more than a single SET, also take other side effects into account. Will it be better that we don't remove the insn if it has side-effect instead of don't record the equiv? This can offer more equiv for later rtl optimization? It's an option I considered, but for stage4 this seems like the safest fix. It's not like this is a very common problem, the effect should be negligible. I see, thanks for the explanation. From my quick test, this patch actually generate code slightly better. The interesting thing I found is if we still initialize the equiv, then a later use in the testcase will be rematerialized, but the equiv constant -1044200508 is forced into memory, thus the rematerialization is a load from memory. Instead, if we don't initialize the equiv then the register contains the reusable value will be kept alive. Bernd
Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn
On 02/12/2016 02:18 PM, Jiong Wang wrote: PR rtl-optimization/69752 * ira.c (update_equiv_regs): When looking for more than a single SET, also take other side effects into account. Will it be better that we don't remove the insn if it has side-effect instead of don't record the equiv? This can offer more equiv for later rtl optimization? It's an option I considered, but for stage4 this seems like the safest fix. It's not like this is a very common problem, the effect should be negligible. Bernd
Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn
On 02/11/2016 06:28 PM, Bernd Schmidt wrote:
This seems fairly straightforward:
(insn 213 455 216 6 (set (reg:SI 266)
(mem/u/c:SI (post_inc:SI (reg/f:SI 267)) [4 S4 A32])) 748
{*thumb1_movsi_insn}
(expr_list:REG_EQUAL (const_int -1044200508 [0xc1c2c3c4])
(expr_list:REG_INC (reg/f:SI 267)
(nil
We don't notice that the SET_SRC has a side effect, record the insn as
an equivalencing one, and later remove it because we replaced the reg
with the constant everywhere. Thus, the increment doesn't take place.
Fixed as follows. Bootstrapped and tested on x86_64-linux. Also compared
before/after dumps for the testcase with arm-elf. Ok?
Bernd
equiv-inc.diff
PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single SET,
also take other side effects into account.
Also note that the reporter says gcc-4.9 didn't have this problem, so
there's a reasonable chance this is a latent regression exposed by
codegen changes prior to IRA.
OK for the trunk.
Jeff
Fix PR69752, insn with REG_INC being removed as equiv_init insn
This seems fairly straightforward:
(insn 213 455 216 6 (set (reg:SI 266)
(mem/u/c:SI (post_inc:SI (reg/f:SI 267)) [4 S4 A32])) 748
{*thumb1_movsi_insn}
(expr_list:REG_EQUAL (const_int -1044200508 [0xc1c2c3c4])
(expr_list:REG_INC (reg/f:SI 267)
(nil
We don't notice that the SET_SRC has a side effect, record the insn as
an equivalencing one, and later remove it because we replaced the reg
with the constant everywhere. Thus, the increment doesn't take place.
Fixed as follows. Bootstrapped and tested on x86_64-linux. Also compared
before/after dumps for the testcase with arm-elf. Ok?
Bernd
PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single SET,
also take other side effects into account.
Index: gcc/ira.c
===
--- gcc/ira.c (revision 233364)
+++ gcc/ira.c (working copy)
@@ -3392,7 +3392,8 @@ update_equiv_regs (void)
/* If this insn contains more (or less) than a single SET,
only mark all destinations as having no known equivalence. */
- if (set == NULL_RTX)
+ if (set == NULL_RTX
+ || side_effects_p (SET_SRC (set)))
{
note_stores (PATTERN (insn), no_equiv, NULL);
continue;
