[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #22 from ubizjak at gmail dot com 2010-01-07 08:02 ---
(In reply to comment #21)
Because at the point of propagation, propagated constant _is_ equal to
whatever REG_EQUAL says. Removing this note at the point of propagation
would IMO disable much more optimization opportunities.
What kind of opportunities exactly? The insn is more precise that the note.
I'm thinking about the same situation with cse2, where constant assignment
(with its REG_EQUAL note) would match another assignment with the same
REG_EQUAL note. cse2 can equal this other assignment (through matching
REG_EQUAL notes) to the constant - actually the same thing that happens in our
problematic case. Since this transformation would happen in the same BB, it
would be perfectly valid - and disabled by removing REG_EQUAL note on constant.
BTW: This fixup happens in very rare occasions. Although the loop looks
scary,
it usually processes very small BBs (I didn't found the case where more than
one assignment was moved at the top of the test BB.
Yes, but removing REG_EQUAL notes pointing to constants seems a bad idea.
Sure, but after the assigment was moved, REG_EQUAL points to invalid
assignment, see this part from the dumps:
39 r78:DI=zero_extend(r154:QI)
REG_DEAD: r154:QI
REG_EQUAL: zero_extend([r152:DI])
583 r82:DI=0x0 here
REG_EQUAL: zero_extend([r152:DI])
40 pc={(r78:DI==0x0)?L230:pc}
REG_BR_PROB: 0x1388
The proposed change removes REG_EQUAL note only on moved insn, (insn 538) in
our case.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #23 from ebotcazou at gcc dot gnu dot org 2010-01-07 08:22
---
I'm thinking about the same situation with cse2, where constant assignment
(with its REG_EQUAL note) would match another assignment with the same
REG_EQUAL note. cse2 can equal this other assignment (through matching
REG_EQUAL notes) to the constant - actually the same thing that happens in our
problematic case. Since this transformation would happen in the same BB, it
would be perfectly valid - and disabled by removing REG_EQUAL note on
constant.
This mechanism very likely results in a bug if the 2 insns are not originally
in the same BB, like in the case at hand. So, assuming they are originally in
the same BB and carry the same REG_EQUAL note, the constant will be propagated
in both insns.
Sure, but after the assigment was moved, REG_EQUAL points to invalid
assignment, see this part from the dumps:
39 r78:DI=zero_extend(r154:QI)
REG_DEAD: r154:QI
REG_EQUAL: zero_extend([r152:DI])
583 r82:DI=0x0 here
REG_EQUAL: zero_extend([r152:DI])
40 pc={(r78:DI==0x0)?L230:pc}
REG_BR_PROB: 0x1388
The proposed change removes REG_EQUAL note only on moved insn, (insn 538) in
our case.
That's too aggressive in the general case, no need to remove a REG_EQUAL note
pointing to a constant if the SRC is also function_invariant_p. An acceptable
compromise could be to remove the note only if its content is also not itself
function_invariant_p.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #24 from ubizjak at gmail dot com 2010-01-07 08:50 --- (In reply to comment #23) The proposed change removes REG_EQUAL note only on moved insn, (insn 538) in our case. That's too aggressive in the general case, no need to remove a REG_EQUAL note pointing to a constant if the SRC is also function_invariant_p. An acceptable compromise could be to remove the note only if its content is also not itself function_invariant_p. Thanks, I will amend the patch as you suggested and send the patch to gcc-patc...@. -- ubizjak at gmail dot com changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |ubizjak at gmail dot com |dot org | Status|NEW |ASSIGNED Last reconfirmed|2010-01-05 12:29:58 |2010-01-07 08:50:35 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #25 from ubizjak at gmail dot com 2010-01-07 09:17 --- New patch revision in testing: --cut here-- Index: ifcvt.c === --- ifcvt.c (revision 155686) +++ ifcvt.c (working copy) @@ -4087,7 +4087,8 @@ dead_or_predicable (basic_block test_bb, if (! note) continue; set = single_set (insn); - if (!set || !function_invariant_p (SET_SRC (set))) + if ((set !function_invariant_p (SET_SRC (set))) + || !function_invariant_p (XEXP (note, 0))) remove_note (insn, note); } while (insn != end (insn = NEXT_INSN (insn))); --cut here-- -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #26 from ubizjak at gmail dot com 2010-01-07 09:23 --- Oops, brain dump error. This is correct: Index: ifcvt.c === --- ifcvt.c (revision 155686) +++ ifcvt.c (working copy) @@ -4087,7 +4087,8 @@ dead_or_predicable (basic_block test_bb, if (! note) continue; set = single_set (insn); - if (!set || !function_invariant_p (SET_SRC (set))) + if (!set || !function_invariant_p (SET_SRC (set)) + || !function_invariant_p (XEXP (note, 0))) remove_note (insn, note); } while (insn != end (insn = NEXT_INSN (insn))); -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #27 from ubizjak at gmail dot com 2010-01-07 11:21 --- Patch at http://gcc.gnu.org/ml/gcc-patches/2010-01/msg00318.html . -- ubizjak at gmail dot com changed: What|Removed |Added URL||http://gcc.gnu.org/ml/gcc- ||patches/2010- ||01/msg00318.html http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #28 from debian-gcc at lists dot debian dot org 2010-01-07 11:54 --- the bootstrap succeeds with the patch from comment #18 applied, test results at http://gcc.gnu.org/ml/gcc-testresults/2010-01/msg00633.html Matthias -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #29 from uros at gcc dot gnu dot org 2010-01-07 13:02 --- Subject: Bug 42511 Author: uros Date: Thu Jan 7 13:02:34 2010 New Revision: 155691 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=155691 Log: PR target/42511 * ifcvt.c (dead_or_predicable): Also remove REG_EQUAL note when note itself is not function_invariant_p. Modified: trunk/gcc/ChangeLog trunk/gcc/ifcvt.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #30 from rguenth at gcc dot gnu dot org 2010-01-07 14:44 --- Fixed. -- rguenth at gcc dot gnu dot org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #31 from uros at gcc dot gnu dot org 2010-01-07 15:00 --- Subject: Bug 42511 Author: uros Date: Thu Jan 7 14:59:59 2010 New Revision: 155693 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=155693 Log: * ifcvt.c (if_convert): Output slim multiple dumps with TDF_SLIM. PR target/42511 * ifcvt.c (dead_or_predicable): Also remove REG_EQUAL note when note itself is not function_invariant_p. Modified: branches/gcc-4_4-branch/gcc/ChangeLog branches/gcc-4_4-branch/gcc/ifcvt.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #32 from uros at gcc dot gnu dot org 2010-01-07 17:32 --- Subject: Bug 42511 Author: uros Date: Thu Jan 7 17:31:43 2010 New Revision: 155698 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=155698 Log: * ifcvt.c (if_convert): Output slim multiple dumps with TDF_SLIM. PR target/42511 * ifcvt.c (dead_or_predicable): Also remove REG_EQUAL note when note itself is not function_invariant_p. Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/ifcvt.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #33 from ubizjak at gmail dot com 2010-01-08 07:33 --- *** Bug 42619 has been marked as a duplicate of this bug. *** -- ubizjak at gmail dot com changed: What|Removed |Added CC||ro at gcc dot gnu dot org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
-- rguenth at gcc dot gnu dot org changed: What|Removed |Added Component|bootstrap |rtl-optimization Keywords||build Summary|bootstrap error in stage3 on|[4.5 Regression] bootstrap |alpha-linux-gnu |error in stage3 on alpha- ||linux-gnu Target Milestone|--- |4.5.0 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #16 from ubizjak at gmail dot com 2010-01-06 15:17 ---
The problem turns out to be quite complex interaction between cse1, cprop3 and
cse2 pass.
Let's start with this RTL dump for from:
tree-ssa-loop-im.i.148r.subreg1
;; Function determine_max_movement (determine_max_movement)
...
L34:
35 NOTE_INSN_BASIC_BLOCK
36 [r73:DI]=r69:DI
37 r155:SI=[r152:DI]
38 r154:QI#0=zero_extract(r155:SI#0,0x8,0x0)
39 r78:DI=zero_extend(r154:QI)
REG_EQUAL: zero_extend([r152:DI])
40 pc={(r78:DI==0x0)?L579:pc}
REG_BR_PROB: 0x1388
...
L571:
572 NOTE_INSN_BASIC_BLOCK
573 r351:SI=[r152:DI]
574 r350:QI#0=zero_extract(r351:SI#0,0x8,0x0)
575 r82:DI=zero_extend(r350:QI)
REG_EQUAL: zero_extend([r152:DI])
L579:
580 NOTE_INSN_BASIC_BLOCK
581 r353:SI=[r152:DI]
582 r352:QI#0=zero_extract(r353:SI#0,0x8,0x0)
583 r82:DI=zero_extend(r352:QI)
REG_EQUAL: zero_extend([r152:DI])
Please note REG_EQUALS in (insn 39) and (insn 583).
Next, cse1 does its job and figures that flow jumps to L579 only when [r152:DI]
is zero. This zero is also propagated to r82 in (insn 583):
tree-ssa-loop-im.i.150r.cse1:
;; Function determine_max_movement (determine_max_movement)
...
L34:
35 NOTE_INSN_BASIC_BLOCK
36 [r73:DI]=r69:DI
37 r155:SI=[r152:DI]
38 r154:QI#0=zero_extract(r155:SI#0,0x8,0x0)
39 r78:DI=zero_extend(r154:QI)
REG_EQUAL: zero_extend([r152:DI])
40 pc={(r78:DI==0x0)?L579:pc}
REG_BR_PROB: 0x1388
...
L571:
572 NOTE_INSN_BASIC_BLOCK
573 r351:SI=r155:SI
574 r350:QI#0=zero_extract(r155:SI#0,0x8,0x0)
575 r82:DI=r78:DI
REG_EQUAL: zero_extend([r152:DI])
L579:
580 NOTE_INSN_BASIC_BLOCK
581 r353:SI=r155:SI
582 r352:QI#0=zero_extract(r155:SI#0,0x8,0x0)
583 r82:DI=0x0
REG_EQUAL: zero_extend([r152:DI])
After all passes, we find following in cprop3 dump:
tree-ssa-loop-im.i.168r.cprop3:
;; Function determine_max_movement (determine_max_movement)
...
L34:
35 NOTE_INSN_BASIC_BLOCK
36 [r73:DI]=r69:DI
REG_DEAD: r69:DI
37 r355:SI=[r152:DI]
38 r154:QI#0=zero_extract(r355:SI#0,0x8,0x0)
39 r78:DI=zero_extend(r154:QI)
REG_DEAD: r154:QI
REG_EQUAL: zero_extend([r152:DI])
583 r82:DI=0x0
REG_EQUAL: zero_extend([r152:DI])
40 pc={(r78:DI==0x0)?L230:pc}
REG_BR_PROB: 0x1388
...
L230:
231 NOTE_INSN_BASIC_BLOCK
232 r104:DI=sign_extend([r73:DI+0x18])
233 r207:DI=r82:DI==0x1
633 r356:DI=leu(r82:DI,0x5)
234 pc={(r207:DI==0x0)?L241:pc}
REG_DEAD: r207:DI
REG_BR_PROB: 0x1ae8
235 NOTE_INSN_BASIC_BLOCK
236 r209:DI=`compiler_params'
237 r208:DI=[r209:DI]
REG_DEAD: r209:DI
REG_EQUAL: [`compiler_params']
238 r107:DI=sign_extend([r208:DI+0x748])
REG_DEAD: r208:DI
...
since both, r78 and r82 equal to the same location, cse2 wisely determines that
both are equal to zero and removes all blocks from the conditional jump onward.
Things go down the drain from here.
tree-ssa-loop-im.i.169r.cse2:
...
L34:
35 NOTE_INSN_BASIC_BLOCK
36 [r73:DI]=r69:DI
REG_DEAD: r69:DI
37 r355:SI=[r152:DI]
38 r154:QI#0=zero_extract(r355:SI#0,0x8,0x0)
583 r82:DI=0x0
REG_EQUAL: zero_extend([r152:DI])
232 r104:DI=sign_extend([r73:DI+0x18])
233 r207:DI=r82:DI==0x1
633 r356:DI=leu(r82:DI,0x5)
234 pc={(r207:DI==0x0)?L241:pc}
REG_DEAD: r207:DI
REG_BR_PROB: 0x1ae8
235 NOTE_INSN_BASIC_BLOCK
236 r209:DI=`compiler_params'
237 r208:DI=[r209:DI]
REG_DEAD: r209:DI
REG_EQUAL: [`compiler_params']
238 r107:DI=sign_extend([r208:DI+0x748])
REG_DEAD: r208:DI
...
So, it looks to me, that when gcc figures a constant in one arm of IF
expression, it should either:
a) remove REG_EQUAL expr when constant is propagated, since this constant
depends on the location of the insn
b) remove REG_EQUAL when insn is hoisted, for the same reason.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #17 from ubizjak at gmail dot com 2010-01-06 18:14 --- This bug is similar (or even a duplicate of) PR21767. ifcvt.c has a fixup code for cases like this, grep ifcvt.c for: /* PR 21767: When moving insns above a conditional branch, REG_EQUAL notes might become invalid. */ So, let's analyse why this fixup doesn't trigger for attached testcase. -- ubizjak at gmail dot com changed: What|Removed |Added BugsThisDependsOn||21767 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #18 from ubizjak at gmail dot com 2010-01-06 23:16 ---
Following patch changes the fix from PR21767 to remove REG_EQUAL notes from all
moved instructions, not only from ones that have non-function-invariant
sources.
--cut here--
Index: ifcvt.c
===
--- ifcvt.c (revision 155681)
+++ ifcvt.c (working copy)
@@ -4079,15 +4079,12 @@ dead_or_predicable (basic_block test_bb,
insn = head;
do
{
- rtx note, set;
+ rtx note;
if (! INSN_P (insn))
continue;
note = find_reg_note (insn, REG_EQUAL, NULL_RTX);
- if (! note)
- continue;
- set = single_set (insn);
- if (!set || !function_invariant_p (SET_SRC (set)))
+ if (note)
remove_note (insn, note);
} while (insn != end (insn = NEXT_INSN (insn)));
--cut here--
Matthias, can you please test this patch if it fixes bootstrap for you?
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #19 from ebotcazou at gcc dot gnu dot org 2010-01-07 00:00 --- Following patch changes the fix from PR21767 to remove REG_EQUAL notes from all moved instructions, not only from ones that have non-function-invariant sources. This seems like a tad aggressive. Why not remove the REG_EQUAL note after the constant propagation has happened instead? -- ebotcazou at gcc dot gnu dot org changed: What|Removed |Added CC||ebotcazou at gcc dot gnu dot ||org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #20 from ubizjak at gmail dot com 2010-01-07 07:29 --- (In reply to comment #19) Following patch changes the fix from PR21767 to remove REG_EQUAL notes from all moved instructions, not only from ones that have non-function-invariant sources. This seems like a tad aggressive. Why not remove the REG_EQUAL note after the constant propagation has happened instead? Because at the point of propagation, propagated constant _is_ equal to whatever REG_EQUAL says. Removing this note at the point of propagation would IMO disable much more optimization opportunities. BTW: This fixup happens in very rare occasions. Although the loop looks scary, it usually processes very small BBs (I didn't found the case where more than one assignment was moved at the top of the test BB. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
[Bug rtl-optimization/42511] [4.5 Regression] bootstrap error in stage3 on alpha-linux-gnu
--- Comment #21 from ebotcazou at gcc dot gnu dot org 2010-01-07 07:39 --- Because at the point of propagation, propagated constant _is_ equal to whatever REG_EQUAL says. Removing this note at the point of propagation would IMO disable much more optimization opportunities. What kind of opportunities exactly? The insn is more precise that the note. BTW: This fixup happens in very rare occasions. Although the loop looks scary, it usually processes very small BBs (I didn't found the case where more than one assignment was moved at the top of the test BB. Yes, but removing REG_EQUAL notes pointing to constants seems a bad idea. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42511
