| Message ID | 58188272.3040104@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 1, 2016 at 12:54 PM, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > Hi all, > > Store merging ICEs on this invalid testcase because it trips up on the > negative bitposition to store to. > It doesn't really expect to handle negative offsets and I believe they won't > occur very often in valid code anyway. > Filling out structs/bitfields/class members involves positive offsets. > I can look into removing all the assumptions about positive offsets if folks > want me to (should involve removing > some 'unsigned' modifiers from HOST_WIDE_INTs and double-checking some range > checks), but for now > this patch just fixes the ICE by rejecting negative offsets early on. > > Bootstrapped and tested on aarch64-none-linux-gnu and x86_64. > > Ok for trunk? Ok (an improvement would be to only reject it after eventually processing a MEM_REF base_addr). Richard. > Thanks, > Kyrill > > 2016-11-01 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR tree-optimization/78162 > * gimple-ssa-store-merging.c (execute): Mark stores with bitpos < 0 > as invalid. > > 2016-11-01 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR tree-optimization/78162 > * gcc.c-torture/compile/pr78162.c: New test.
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c index f82cad35afbc10eea76957d38100acdce137d271..4dbf03f61ab5ed4e33481d53cc3c2dee1a457f8f 100644 --- a/gcc/gimple-ssa-store-merging.c +++ b/gcc/gimple-ssa-store-merging.c @@ -1371,7 +1371,7 @@ pass_store_merging::execute (function *fun) &unsignedp, &reversep, &volatilep); /* As a future enhancement we could handle stores with the same base and offset. */ - bool invalid = offset || reversep + bool invalid = offset || reversep || bitpos < 0 || ((bitsize > MAX_BITSIZE_MODE_ANY_INT) && (TREE_CODE (rhs) != INTEGER_CST)) || !rhs_valid_for_store_merging_p (rhs) diff --git a/gcc/testsuite/gcc.c-torture/compile/pr78162.c b/gcc/testsuite/gcc.c-torture/compile/pr78162.c new file mode 100644 index 0000000000000000000000000000000000000000..743d4e678b5b8c9a7ee6fb45254f86212523d59c --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr78162.c @@ -0,0 +1,10 @@ +/* PR tree-optimization/78162. + Handle negative offsets in store merging gracefully. */ + +int a, b[1][2]; + +void fn1() +{ + for (a = 0; a < 2; a++) + b[-1][a] = 0; +}
Hi all, Store merging ICEs on this invalid testcase because it trips up on the negative bitposition to store to. It doesn't really expect to handle negative offsets and I believe they won't occur very often in valid code anyway. Filling out structs/bitfields/class members involves positive offsets. I can look into removing all the assumptions about positive offsets if folks want me to (should involve removing some 'unsigned' modifiers from HOST_WIDE_INTs and double-checking some range checks), but for now this patch just fixes the ICE by rejecting negative offsets early on. Bootstrapped and tested on aarch64-none-linux-gnu and x86_64. Ok for trunk? Thanks, Kyrill 2016-11-01 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR tree-optimization/78162 * gimple-ssa-store-merging.c (execute): Mark stores with bitpos < 0 as invalid. 2016-11-01 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR tree-optimization/78162 * gcc.c-torture/compile/pr78162.c: New test.