This is the mail archive of the
gcc-bugs@gcc.gnu.org
mailing list for the GCC project.
[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4
- From: "amker.cheng at gmail dot com" <gcc-bugzilla at gcc dot gnu dot org>
- To: gcc-bugs at gcc dot gnu dot org
- Date: Tue, 18 Mar 2014 10:08:06 +0000
- Subject: [Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4
- Auto-submitted: auto-generated
- References: <bug-60363-4 at http dot gcc dot gnu dot org/bugzilla/>
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363
--- Comment #8 from bin.cheng <amker.cheng at gmail dot com> ---
Here is the findings.
1) After patching 208165, there are two more jump threading opportunities for
dom1 pass. Jump threading is doing alright, the interesting thing is why there
is no such opportunities before patching. The two additional opportunities are
introduced after vrp1.
VRP1 dump without patch:
<bb 2>:
goto <bb 15>;
<bb 3>:
if (b_elt_11(D) != 0B)
goto <bb 4>;
else
goto <bb 8>;
<bb 4>:
goto <bb 6>;
<bb 5>:
kill_elt_14 = kill_elt_2->next;
<bb 6>:
# kill_elt_2 = PHI <kill_elt_4(4), kill_elt_14(5)>
if (kill_elt_2 != 0B)
goto <bb 7>;
else
goto <bb 19>;
<bb 7>:
_12 = kill_elt_2->indx;
_13 = b_elt_11(D)->indx;
if (_12 < _13)
goto <bb 5>;
else
goto <bb 18>;
<bb 8>:
# kill_elt_3 = PHI <kill_elt_4(3)>
if (b_elt_11(D) != 0B)
goto <bb 9>;
else
goto <bb 14>;
<bb 9>:
if (kill_elt_3 != 0B)
goto <bb 10>;
else
goto <bb 14>;
<bb 10>:
# kill_elt_32 = PHI <kill_elt_3(9), kill_elt_39(18)>
_15 = kill_elt_32->indx;
_16 = b_elt_11(D)->indx;
if (_15 == _16)
goto <bb 11>;
else
goto <bb 14>;
<bb 11>:
if (a_elt_9(D) == 0B)
goto <bb 13>;
else
goto <bb 12>;
<bb 12>:
_17 = a_elt_9(D)->indx;
if (_16 <= _17)
goto <bb 13>;
else
goto <bb 14>;
<bb 13>:
_20 = (int) changed_1;
_25 = bitmap_elt_ior (dst_21(D), dst_elt_22(D), dst_prev_23(D), a_elt_9(D),
&tmp_elt, _20);
changed_26 = (unsigned char) _25;
tmp_elt ={v} {CLOBBER};
<bb 14>:
# changed_18 = PHI <changed_26(13), changed_1(8), changed_1(9),
changed_1(10), changed_1(12), changed_1(18), changed_1(19)>
# kill_elt_33 = PHI <kill_elt_32(13), kill_elt_3(8), kill_elt_3(9),
kill_elt_32(10), kill_elt_32(12), kill_elt_39(18), kill_elt_35(19)>
<bb 15>:
# changed_1 = PHI <changed_18(14), 0(2)>
# kill_elt_4 = PHI <kill_elt_33(14), kill_elt_7(D)(2)>
if (a_elt_9(D) != 0B)
goto <bb 3>;
else
goto <bb 16>;
<bb 16>:
if (b_elt_11(D) != 0B)
goto <bb 3>;
else
goto <bb 17>;
<bb 17>:
# changed_6 = PHI <changed_1(16)>
changed_28 = changed_6;
return changed_28;
<bb 18>:
# kill_elt_39 = PHI <kill_elt_2(7)>
if (b_elt_11(D) != 0B)
goto <bb 10>;
else
goto <bb 14>;
<bb 19>:
# kill_elt_35 = PHI <0B(6)>
goto <bb 14>;
VRP1 dump with patch:
<bb 2>:
goto <bb 15>;
<bb 3>:
if (b_elt_11(D) != 0B)
goto <bb 4>;
else
goto <bb 8>;
<bb 4>:
# kill_elt_10 = PHI <kill_elt_4(3)>
goto <bb 6>;
<bb 5>:
kill_elt_14 = kill_elt_2->next;
<bb 6>:
# kill_elt_2 = PHI <kill_elt_10(4), kill_elt_14(5)>
if (kill_elt_2 != 0B)
goto <bb 7>;
else
goto <bb 19>;
<bb 7>:
_12 = kill_elt_2->indx;
_13 = b_elt_11(D)->indx;
if (_12 < _13)
goto <bb 5>;
else
goto <bb 20>;
<bb 8>:
# kill_elt_3 = PHI <kill_elt_4(3)>
if (b_elt_11(D) != 0B)
goto <bb 9>;
else
goto <bb 14>;
<bb 9>:
if (kill_elt_3 != 0B)
goto <bb 10>;
else
goto <bb 14>;
<bb 10>:
# kill_elt_34 = PHI <kill_elt_3(9), kill_elt_37(20)>
_15 = kill_elt_34->indx;
_16 = b_elt_11(D)->indx;
if (_15 == _16)
goto <bb 11>;
else
goto <bb 14>;
<bb 11>:
if (a_elt_9(D) == 0B)
goto <bb 13>;
else
goto <bb 12>;
<bb 12>:
_17 = a_elt_9(D)->indx;
if (_16 <= _17)
goto <bb 13>;
else
goto <bb 14>;
<bb 13>:
_20 = (int) changed_1;
_25 = bitmap_elt_ior (dst_21(D), dst_elt_22(D), dst_prev_23(D), a_elt_9(D),
&tmp_elt, _20);
changed_26 = (unsigned char) _25;
tmp_elt ={v} {CLOBBER};
<bb 14>:
# changed_19 = PHI <changed_1(8), changed_1(18), changed_26(13),
changed_1(12), changed_1(10), changed_1(19), changed_1(20), changed_1(9)>
# kill_elt_18 = PHI <kill_elt_3(8), 0B(18), kill_elt_34(13), kill_elt_34(12),
kill_elt_34(10), kill_elt_41(19), kill_elt_37(20), 0B(9)>
<bb 15>:
# changed_1 = PHI <changed_19(14), 0(2)>
# kill_elt_4 = PHI <kill_elt_18(14), kill_elt_7(D)(2)>
if (a_elt_9(D) != 0B)
goto <bb 3>;
else
goto <bb 16>;
<bb 16>:
if (b_elt_11(D) != 0B)
goto <bb 3>;
else
goto <bb 17>;
<bb 17>:
# changed_29 = PHI <changed_1(16)>
changed_28 = changed_29;
return changed_28;
<bb 18>:
goto <bb 14>;
<bb 19>:
# kill_elt_41 = PHI <0B(6)>
if (b_elt_11(D) != 0B)
goto <bb 18>;
else
goto <bb 14>;
<bb 20>:
# kill_elt_37 = PHI <kill_elt_2(7)>
if (b_elt_11(D) != 0B)
goto <bb 10>;
else
goto <bb 14>;
The problem part is as below:
<bb 14>:
# changed_19 = PHI <changed_1(8), changed_1(18), changed_26(13),
changed_1(12), changed_1(10), changed_1(19), changed_1(20), changed_1(9)>
# kill_elt_18 = PHI <kill_elt_3(8), 0B(18), kill_elt_34(13), kill_elt_34(12),
kill_elt_34(10), kill_elt_41(19), kill_elt_37(20), 0B(9)>
...
<bb 18>:
goto <bb 14>;
<bb 19>:
# kill_elt_41 = PHI <0B(6)>
if (b_elt_11(D) != 0B)
goto <bb 18>;
else
goto <bb 14>;
Since PHI argument "kill_elt_41(19)" equals to "0B(18)", the bb18 is a
removable forward basic block. By removing bb18, bb19 can be simplified into a
single jump to bb14 too. This is exactly what GCC does without patch.
When CFGcleanup tries to remove bb18, it has to check that PHI arguments
"kill_elt_41(19)" and "0B(18)" are compatible. Unfortunately, CFGcleanup
doesn't evaluate value of ssa_name and it doesn't know that "kill_elt_41(19)"
equals to "0" along jump threading path "19->14".
Since jump threading is now a flow sensitive optimization, it knows very well
about that truth, so I come up below conclusion:
Jump threading adds PHI argument by copying existing one when duplicating basic
blocks along jump threading path. Itâs very possible that a PHI argument has a
constant value along threading path, but jump threading doesnât do any value
evaluation and just copies the ssa_name. If we copy the constant value instead
of ssa_name, cfgcleanup after jump threading can see more removable forward
basic blocks thus the issue is fixed.
For this specific case, jump threading should be able to add argument "0(19)"
rather than "kill_elt_41(19)" to bb14, like:
<bb 14>:
# changed_19 = PHI <changed_1(8), changed_1(18), changed_26(13),
changed_1(12), changed_1(10), changed_1(19), changed_1(20), changed_1(9)>
# kill_elt_18 = PHI <kill_elt_3(8), 0B(18), kill_elt_34(13), kill_elt_34(12),
kill_elt_34(10), 0(19), kill_elt_37(20), 0B(9)>
I worked out a patch fixing the issue, will send it for review.