On Mon, Nov 28, 2016 at 2:26 PM, Robin Dapp <rd...@linux.vnet.ibm.com> wrote: >>> + /* Sign-extend @1 to TYPE. */ >>> + w1 = w1.from (w1, TYPE_PRECISION (type), SIGNED); >>> >>> not sure why you do always sign-extend. If the inner op is unsigned >>> and we widen then that's certainly bogus considering your UINT_MAX >>> example above. Does >>> >>> w1 = w1.from (w1, TYPE_PRECISION (type), TYPE_SIGN >>> (inner_type)); >>> >>> not work for some reason? > > With TYPE_SIGN (inner_type), I encountered situations like this in the > testsuite (mostly Fortran but also 20000422-1.c): > > Folding statement: _1 = _9 + 1; > Applying pattern match.pd:1214, gimple-match.c:8719 > gimple_simplified to _93 = (sizetype) _8; > _1 = _93 + 4294967296; > Folded into: _1 = _93 + 4294967296; > > in > _8 = (unsigned int) i_73; > _5 = _8 + 4294967295; > _9 = (sizetype) _5; > _93 = (sizetype) _8; > _1 = _93 + 4294967296; > > TYPE_SIGN (inner_type) is PLUS here, although it should be MINUS in > order to combine -1 and +1 to 0. Perhaps this can be handled differently > with keeping TYPE_SIGN (inner_type)?
So we have (uint64_t)(uint32 + -1U) + 1 and using TYPE_SIGN (inner_type) produces (uint64_t)uint32 + -1U + 1. This simply means that we cannot ignore overflow of the inner operation and for some reason your change to extract_range_from_binary_expr didn't catch this. That is _8 + 4294967295 overflows but we ignored that. > On the other hand, using SIGNED instead of TYPE_SIGN (inner_type) worked > for all cases I looked at, like > if (a > 1 && a < 10) > return (unsigned long)(a + UINT_MAX) + 1; > For > if (a > 0 && a < 10) > extract_range...() would not find a non-VR_VARYING range although we > should probably still be able to simplify this. > > if (a > 0) > Here, vrp figures out [0,4294967294] and the simplification takes place. > > For > if (a <= 10) > return (unsigned long)(a + (UINT_MAX - 10)) + 1; > 003t.original already reads > return (long unsigned int) a + 4294967286 > > From this I hand-wavingly deduced, we'd only see instances where a > sign-extension of the constant is fine (test suites on s390x and x86 > affirm this for what it's woth) but I don't have a cogent reason hence > my doubts :) Well, even if sign-extending you can probably construct a testcase where that's still wrong with respect to overflow. Richard. > I'm ok with omitting the sign-changing case (I hadn't though of it > anyway) and adapted the patch. I haven't attached the updated version > though, because I'm still unsure about the issue above (despite the > clean test suite). > > Regards > Robin >
