From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x234.google.com (mail-lj1-x234.google.com [IPv6:2a00:1450:4864:20::234]) by sourceware.org (Postfix) with ESMTPS id 227D03858CDB for ; Wed, 24 May 2023 11:18:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 227D03858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x234.google.com with SMTP id 38308e7fff4ca-2af2d092d7aso10566391fa.2 for ; Wed, 24 May 2023 04:18:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684927079; x=1687519079; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=z/vH221FXLUzJZ06zN0vzUep319J4kuAz/8dL21aFOE=; b=oBSUTwmyJWxC0EP1c5vWo1b/6ZAyU+xAMd5rM10yk6Tz5dE6TMRx0SGGwVWiiiQpO+ 4/aGl/OPP2YJXCJrcSUXGBfi+HNscrZ6Ttqki/PW+7mGqHJOyLj0utmuppGRZPEMhTG2 /cnLjK2Vb3a3KA9r7trw+SjGRipYBeOY49WHXa+v1W+cs+SIC6n9bOS6c2/8niv/7qGq s1qsb8L6MuDQU6optMPYl2t+sxVGzPMCCwPEyD/WhKahOY6vQZVvNDSi1MjcxlLQ3m5L +fnwNbzvuGqVG24+v9pQNuG1lTulaJ1emnaJB/mbhrSjRAyr3SBVDk4QjAGW80zs9KW2 Ng5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684927079; x=1687519079; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=z/vH221FXLUzJZ06zN0vzUep319J4kuAz/8dL21aFOE=; b=UvCVzX//DpVD+P7PXjdMbMr8zyup1hAi8o5c6QX6lNs3kd7nwVJgpE9m+WRpriTZ4s dx51Y3tDlFEEdfqd8XIhwLI3RZE+zBiWWSxtVvt/6mIZ4AVLgw07vmJmHrJeeHgv0pWd odJEyNqo0pmOHE9DiSB/EqfLTPOK/BUSI65/c9j2WIhMhKhmHec2XUr42WdkCwyp8dbC TwWwehZXxXs7Ker4zaJAh8MVWndxU7ysRUDhrVBtpurvR3r4jbWtR+QB1pXBigoweVkF Yddfy/8aaHz33VhWIK2qKEucbedWzWBrw2fz2s7PkgiEVn3poElfQdBTbmu5IMy91SCa oZ5Q== X-Gm-Message-State: AC+VfDxXFTyrL5SAzijaTJRI9xrr7YCYmPnF/Tx0ZJCJI1Nket/9itAS LQqJQJcYg/UhTOiPFkZo8Qhi0NAkDacQWXvPjQda2bNLKRo= X-Google-Smtp-Source: ACHHUZ6Z6W6eiU4pO4RRc7Zdhq6oc6SDSUSkbs0Ii5n0OnAGIqHIc6W3woiAif2EwY6D6W18wMotQXXpPbizat+bhOQ= X-Received: by 2002:a2e:7d11:0:b0:2af:2d77:9be1 with SMTP id y17-20020a2e7d11000000b002af2d779be1mr4741008ljc.6.1684927079121; Wed, 24 May 2023 04:17:59 -0700 (PDT) MIME-Version: 1.0 References: <2883909.e9J7NaK4W3@fomalhaut> In-Reply-To: <2883909.e9J7NaK4W3@fomalhaut> From: Richard Biener Date: Wed, 24 May 2023 13:15:52 +0200 Message-ID: Subject: Re: [PATCH] Fix artificial overflow during GENERIC folding To: Eric Botcazou Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, May 24, 2023 at 11:56=E2=80=AFAM Eric Botcazou via Gcc-patches 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 ru= n > time [enabled by default] > > This directly comes from the GENERIC folding setting a bogus TREE_OVERFLO= W 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) <=3D element_precision (TREE_TYPE (@1= ))) > (with { tree utype =3D unsigned_type_for (type); } > (convert (negate (convert:utype @1)))) > (if (element_precision (type) <=3D 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=3Dsize_t, A=3D(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) <=3D element_precision (TREE_TYPE (@1= ))) > (with { tree utype =3D unsigned_type_for (type); } > (convert (negate (convert:utype @1)))) > (if (element_precision (type) <=3D 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) =3D=3D INTEGER_CST > && tree_int_cst_sign_bit (@1) =3D=3D 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-product= ive > 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 t= he > 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 =3D NULL_TREE; if (res_op->code.is_tree_code ()) { auto code =3D tree_code (res_op->code); if (IS_EXPR_CODE_CLASS (TREE_CODE_CLASS (code)) && TREE_CODE_LENGTH (code) =3D=3D 1) tem =3D const_unop (code, res_op->type, res_op->ops[0]); } else tem =3D fold_const_call (combined_fn (res_op->code), res_op->type, res_op->ops[0]); if (tem !=3D NULL_TREE && CONSTANT_CLASS_P (tem)) { if (TREE_OVERFLOW_P (tem)) tem =3D 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_overflo= w (). 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 > > * match.pd ((T)P - (T)(P + A) -> -(T) A): Avoid artificial overfl= ow > on constants. > > > 2023-05-24 Eric Botcazou > > * gnat.dg/specs/storage_offset1.ads: New test. > > -- > Eric Botcazou