| Message ID | a71404bf-d5f4-f83f-5bd9-c336cd732c64@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 26, 2016 at 5:13 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote: > Hi, > > For testcase in pr71994, type of bb conditional result and the type of the > PHI stmt are different (as om.0_1 is int and the first PHI argument is > _bool; PHI stmt uses a constant zero that comes from edge 2). Therefore when > we optimize final range test stmt, we end up setting integer 1 for other PHI > argument. This results in ICE. > > <bb 2>: > om.0_1 = om; > _2 = om.0_1 >= 0; > int _3 = (int) _2; > if (om.0_1 != 0) > goto <bb 3>; > else > goto <bb 4>; > > <bb 3>: > int _4 = om.0_1 & _3; > _Bool _12 = _4 != 0; > <bb 4>: > > # _Bool _13 = PHI <0(2), _12(3)> > > > IMHO, the fix should be that, we should check the type before replacing the > value (punt otherwise). Attached patch does that. Bootstrapped and > regression tested on x86_64-linux-gnu with no new regressions. Is this OK > for trunk? Ugh, this is undocumented spaghetti code I am not familiar with. I can't see if it is safe to truncate a constant here so I'd feel safer to just punt instead. Richard. > Thanks, > Kugan > > gcc/testsuite/ChangeLog: > > 2016-07-26 Kugan Vivekanandarajah <kuganv@linaro.org> > > * gcc.dg/torture/pr71994.c: New test. > > gcc/ChangeLog: > > 2016-07-26 Kugan Vivekanandarajah <kuganv@linaro.org> > > * tree-ssa-reassoc.c (maybe_optimize_range_tests): Check type > compatibility. >
On 07/25/2016 09:13 PM, kugan wrote: > Hi, > > For testcase in pr71994, type of bb conditional result and the type of > the PHI stmt are different (as om.0_1 is int and the first PHI argument > is _bool; PHI stmt uses a constant zero that comes from edge 2). > Therefore when we optimize final range test stmt, we end up setting > integer 1 for other PHI argument. This results in ICE. > > <bb 2>: > om.0_1 = om; > _2 = om.0_1 >= 0; > int _3 = (int) _2; > if (om.0_1 != 0) > goto <bb 3>; > else > goto <bb 4>; > > <bb 3>: > int _4 = om.0_1 & _3; > _Bool _12 = _4 != 0; > <bb 4>: > > # _Bool _13 = PHI <0(2), _12(3)> > > > IMHO, the fix should be that, we should check the type before replacing > the value (punt otherwise). Attached patch does that. Bootstrapped and > regression tested on x86_64-linux-gnu with no new regressions. Is this > OK for trunk? > > Thanks, > Kugan > > gcc/testsuite/ChangeLog: > > 2016-07-26 Kugan Vivekanandarajah <kuganv@linaro.org> > > * gcc.dg/torture/pr71994.c: New test. > > gcc/ChangeLog: > > 2016-07-26 Kugan Vivekanandarajah <kuganv@linaro.org> > > * tree-ssa-reassoc.c (maybe_optimize_range_tests): Check type > compatibility. I'd kind of like to see some analysis of how we've got a bool here -- ISTM it ought to have been converted it to the type of the LHS of the PHI when propagated. Jeff
diff --git a/gcc/testsuite/gcc.dg/torture/pr71994.c b/gcc/testsuite/gcc.dg/torture/pr71994.c index e69de29..8f5e92c 100644 --- a/gcc/testsuite/gcc.dg/torture/pr71994.c +++ b/gcc/testsuite/gcc.dg/torture/pr71994.c @@ -0,0 +1,14 @@ +/* PR tree-optimization/71994 */ +/* { dg-do compile } */ +int om, h6; + +void eo (void) +{ + const int tl = 1; + int ln; + + h6 = (om + tl) > 0; + ln = om && (om & h6); + h6 = om; + om = ln < h6; +} diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c index 18cf978..91d7f06 100644 --- a/gcc/tree-ssa-reassoc.c +++ b/gcc/tree-ssa-reassoc.c @@ -3635,8 +3635,15 @@ maybe_optimize_range_tests (gimple *stmt) { gcc_assert (bb == last_bb); new_op = ops[bbinfo[idx].first_idx++]->op; + if (!types_compatible_p (TREE_TYPE (new_op), + TREE_TYPE (bbinfo[idx].op)) + && TREE_CODE (new_op) == INTEGER_CST) + new_op = fold_convert (TREE_TYPE (bbinfo[idx].op), + new_op); } - if (bbinfo[idx].op != new_op) + if (bbinfo[idx].op != new_op + && types_compatible_p (TREE_TYPE (new_op), + TREE_TYPE (bbinfo[idx].op))) { imm_use_iterator iter; use_operand_p use_p;
Hi, For testcase in pr71994, type of bb conditional result and the type of the PHI stmt are different (as om.0_1 is int and the first PHI argument is _bool; PHI stmt uses a constant zero that comes from edge 2). Therefore when we optimize final range test stmt, we end up setting integer 1 for other PHI argument. This results in ICE. <bb 2>: om.0_1 = om; _2 = om.0_1 >= 0; int _3 = (int) _2; if (om.0_1 != 0) goto <bb 3>; else goto <bb 4>; <bb 3>: int _4 = om.0_1 & _3; _Bool _12 = _4 != 0; <bb 4>: # _Bool _13 = PHI <0(2), _12(3)> IMHO, the fix should be that, we should check the type before replacing the value (punt otherwise). Attached patch does that. Bootstrapped and regression tested on x86_64-linux-gnu with no new regressions. Is this OK for trunk? Thanks, Kugan gcc/testsuite/ChangeLog: 2016-07-26 Kugan Vivekanandarajah <kuganv@linaro.org> * gcc.dg/torture/pr71994.c: New test. gcc/ChangeLog: 2016-07-26 Kugan Vivekanandarajah <kuganv@linaro.org> * tree-ssa-reassoc.c (maybe_optimize_range_tests): Check type compatibility.