On Wed, May 24, 2023 at 11:56 AM Eric Botcazou via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi, > > on the attached testcase, the Ada compiler gives a bogus warning: > storage_offset1.ads:16:52: warning: Constraint_Error will be raised at run > time [enabled by default] > > This directly comes from the GENERIC folding setting a bogus TREE_OVERFLOW on > an INTEGER_CST during the (T)P - (T)(P + A) -> -(T) A transformation: > > /* (T)P - (T)(P + A) -> -(T) A */ > (simplify > (minus (convert? @0) > (convert (plus:c @@0 @1))) > (if (INTEGRAL_TYPE_P (type) > && TYPE_OVERFLOW_UNDEFINED (type) > && element_precision (type) <= element_precision (TREE_TYPE (@1))) > (with { tree utype = unsigned_type_for (type); } > (convert (negate (convert:utype @1)))) > (if (element_precision (type) <= element_precision (TREE_TYPE (@1)) > /* For integer types, if A has a smaller type > than T the result depends on the possible > overflow in P + A. > E.g. T=size_t, A=(unsigned)429497295, P>0. > However, if an overflow in P + A would cause > undefined behavior, we can assume that there > is no overflow. */ > || (INTEGRAL_TYPE_P (TREE_TYPE (@1)) > && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1)))) > (negate (convert @1))))) > (simplify > (minus (convert @0) > (convert (pointer_plus @@0 @1))) > (if (INTEGRAL_TYPE_P (type) > && TYPE_OVERFLOW_UNDEFINED (type) > && element_precision (type) <= element_precision (TREE_TYPE (@1))) > (with { tree utype = unsigned_type_for (type); } > (convert (negate (convert:utype @1)))) > (if (element_precision (type) <= element_precision (TREE_TYPE (@1)) > /* For pointer types, if the conversion of A to the > final type requires a sign- or zero-extension, > then we have to punt - it is not defined which > one is correct. */ > || (POINTER_TYPE_P (TREE_TYPE (@0)) > && TREE_CODE (@1) == INTEGER_CST > && tree_int_cst_sign_bit (@1) == 0)) > (negate (convert @1))))) > > Ironically enough, this occurs because of the intermediate conversion to an > unsigned type which is supposed to hide overflows, but is counter-productive > for constants because TREE_OVERFLOW is always set for them, so it ends up > setting a bogus TREE_OVERFLOW when converting back to the original type. > > The fix simply redirects INTEGER_CSTs to the other, direct path without the > intermediate conversion to the unsigned type. > > Tested on x86-64/Linux, OK for the mainline?
Hmm. gimple_resimplifyN do if (constant_for_folding (res_op->ops[0])) { tree tem = NULL_TREE; if (res_op->code.is_tree_code ()) { auto code = tree_code (res_op->code); if (IS_EXPR_CODE_CLASS (TREE_CODE_CLASS (code)) && TREE_CODE_LENGTH (code) == 1) tem = const_unop (code, res_op->type, res_op->ops[0]); } else tem = fold_const_call (combined_fn (res_op->code), res_op->type, res_op->ops[0]); if (tem != NULL_TREE && CONSTANT_CLASS_P (tem)) { if (TREE_OVERFLOW_P (tem)) tem = drop_tree_overflow (tem); res_op->set_value (tem); so why doesn't that apply here? Ah, because it's for GENERIC folding and there we use fold_buildN. I don't like littering the patterns with this and it's likely far from the only cases we have? Since we did move some of the patterns from fold-const.cc to match.pd and the frontends might be interested in TREE_OVERFLOW (otherwise we'd just scrap that!) I'm not sure removing the flag is good (and I never was really convinced the setting for the implementation defined behavior on conversion to unsigned is good). I'm also hesitant to invent another syntax, like (convert (negate (convert:utype* @1)))) that would then code-generate a if (TREE_OVERFLOW_P (..)) drop_tree_overflow (). Am I correct that the user writing such a conversion in Ada _should_ get a constraint violation? So it's just the middle-end introducing it to avoid undefined signed overflow that's on error? I'll also note that fold_convert_const_int_from_int shouldn't set TREE_OVERFLOW on unsigned destination types? So it's the outer conversion back to signed that generates the TREE_OVERFLOW? Would it help to use a (view_convert ...) here? For non-constants that should be folded back to a sign changing (convert ...) but the constant folding should hopefully happen earlier? But it's again implementation defined behavior we have here, so not sure we need TREE_OVERFLOW at all. Richard. > > 2023-05-24 Eric Botcazou <ebotca...@adacore.com> > > * match.pd ((T)P - (T)(P + A) -> -(T) A): Avoid artificial overflow > on constants. > > > 2023-05-24 Eric Botcazou <ebotca...@adacore.com> > > * gnat.dg/specs/storage_offset1.ads: New test. > > -- > Eric Botcazou