Hi! On Tue, Jan 26, 2016 at 04:54:43PM +0100, Richard Biener wrote: > > Somehow PR 65656 miscompiled: > > > > if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT) > > ? xi.len == 1 && xi.val[0] >= 0 > > : xi.precision <= HOST_BITS_PER_WIDE_INT) > > > > which turned the expression into true and hit > > > > val[0] = xi.to_uhwi () >> shift; > > result.set_len (1); > > I think we need a better analysis as we use __builtin_constant_p > elsewhere as well.
So, I had a look at this bug and it seems clearly a bug on the wide-int.h side. wi::lrshift right now doesn't do what the comment says (which says that for the larger precision fixed types it only cares about constant shift). Compared to wi::lshift, for the STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT) case lrshift doesn't bother to check the value of shift. While for xi.precision <= HOST_BITS_PER_WIDE_INT, at least conforming code should not perform out of bounds shifts and thus there is no need to check the value of shift, for xi.precision > HOST_BITS_PER_WIDE_INT it is completely valid to have large shift (in between HOST_BITS_PER_WIDE_INT inclusive and xi.precision exclusive), and then we just trigger undefined behavior for that case. The question is, shall we do what wi::lshift does and have the fast path only for the constant shifts, as the untested patch below does, or shall we check shift dynamically (thus use shift < HOST_BITS_PER_WIDE_INT instead of STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT) in the patch), or shall we have another case for such shifts and set val[0] = 0; then? The __builtin_constant_p change affects whether we trigger this bug only for fixed large precision instantiations (with trunk __builtin_constant_p) or also by mistake for variable large precision instantiations (with gcc 5 __builtin_constant_p), but the fixed large precision instantiations are broken in any case. 2016-01-26 Jakub Jelinek <ja...@redhat.com> PR tree-optimization/69399 * wide-int.h (wi::lrshift): For larger precisions, only use fast path if shift is known to be < HOST_BITS_PER_WIDE_INT. --- gcc/wide-int.h.jj 2016-01-04 14:55:50.000000000 +0100 +++ gcc/wide-int.h 2016-01-26 19:05:20.715269366 +0100 @@ -2909,7 +2909,9 @@ wi::lrshift (const T1 &x, const T2 &y) For variable-precision integers like wide_int, handle HWI and sub-HWI integers inline. */ if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT) - ? xi.len == 1 && xi.val[0] >= 0 + ? (STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT) + && xi.len == 1 + && xi.val[0] >= 0) : xi.precision <= HOST_BITS_PER_WIDE_INT) { val[0] = xi.to_uhwi () >> shift; Jakub