From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by sourceware.org (Postfix) with ESMTPS id EEE0C3858C50 for ; Mon, 15 Aug 2022 07:56:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EEE0C3858C50 Received: by mail-ed1-x52b.google.com with SMTP id w3so8660031edc.2 for ; Mon, 15 Aug 2022 00:56:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=q0dXr+7F2nEBTpuA92TItb32K/2k4gqTNB+1ljq2/1A=; b=JMh96LESHRphE0rB4xRBdl/luhNuFftQeMKD3oOZykJYJQWDrFpbXjMnvfJWJ8YAmy xrgiakuspHGjtrbFIuFxK0xCFPFaGg4aMWBds1cnAhhMcK5XoOTKuBoXJcPYsE4WQjCU N/3dVOU7+62kmfl0Qb3V15YHg/swJddhv6WKyPyZpZ+4zMrUrAEYuGEud4Ox9hUGuXRo o0k/TVDJUewquwFabx+rUNDrJDQuC/ucB+y0FEPU4+UmTcICdpz6XKfuOP/XOUAxKDvE 6vyrD4bN0l5ciwV1fdSxGIEuANAcsW69dcUqwPGCVOXwpB4PnAp89qKDv3iza0bhWpSZ D05w== X-Gm-Message-State: ACgBeo21Wz1FUznCmmQRmB0YqCIcklUGcafk8nPke0trFJYwr56ZYtFi GCuJLVXTLpeCFnqC9X4EI5uBHivc/3i/gjZ6KBI= X-Google-Smtp-Source: AA6agR62S5lGiNVFyQ5HUk0/Z4+kJgdOaNBWJc1h9SYF0emSLOcHagEmVsJ4UR9J3yP/uV6n4Ja4ZM+ggXXdj3dhabk= X-Received: by 2002:a05:6402:550c:b0:443:7d15:d57f with SMTP id fi12-20020a056402550c00b004437d15d57fmr7731376edb.147.1660550172723; Mon, 15 Aug 2022 00:56:12 -0700 (PDT) MIME-Version: 1.0 References: <00ce01d8ab06$19113060$4b339120$@nextmovesoftware.com> <00bc01d8ae9b$c86e5ef0$594b1cd0$@nextmovesoftware.com> In-Reply-To: <00bc01d8ae9b$c86e5ef0$594b1cd0$@nextmovesoftware.com> From: Richard Biener Date: Mon, 15 Aug 2022 09:55:59 +0200 Message-ID: Subject: Re: [PATCH] PR tree-optimization/64992: (B << 2) != 0 is B when B is Boolean. To: Roger Sayle Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.2 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Aug 2022 07:56:16 -0000 On Sat, Aug 13, 2022 at 12:35 AM Roger Sayle wrote: > > Hi Richard, > > > -----Original Message----- > > From: Richard Biener > > Sent: 08 August 2022 12:49 > > Subject: Re: [PATCH] PR tree-optimization/64992: (B << 2) != 0 is B when B is > > Boolean. > > > > On Mon, Aug 8, 2022 at 11:06 AM Roger Sayle > > wrote: > > > > > > This patch resolves both PR tree-optimization/64992 and PR > > > tree-optimization/98956 which are missed optimization enhancement > > > request, for which Andrew Pinski already has a proposed solution > > > (related to a fix for PR tree-optimization/98954). Yesterday, I > > > proposed an alternate improved patch for PR98954, which although > > > superior in most respects, alas didn't address this case [which > > > doesn't include a BIT_AND_EXPR], hence this follow-up fix. > > > > > > For many functions, F(B), of a (zero-one) Boolean value B, the > > > expression F(B) != 0 can often be simplified to just B. Hence "(B * > > > 5) != 0" is B, "-B != 0" is B, "bswap(B) != 0" is B, "(B >>r 3) != 0" > > > is B. These are all currently optimized by GCC, with the strange > > > exception of left shifts by a constant (possibly due to the > > > undefined/implementation defined behaviour when the shift constant is > > > larger than the first operand's precision). > > > This patch adds support for this particular case, when the shift > > > constant is valid. > > > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > > > and make -k check, both with and without --target_board=unix{-m32}, > > > with no new failures. Ok for mainline? > > > > +/* (X << C) != 0 can be simplified to X, when X is zero_one_valued_p. > > +*/ (simplify > > + (ne (lshift zero_one_valued_p@0 INTEGER_CST@1) integer_zerop@2) > > + (if (tree_fits_shwi_p (@1) > > + && tree_to_shwi (@1) > 0 > > + && tree_to_shwi (@1) < TYPE_PRECISION (TREE_TYPE (@0))) > > + (convert @0))) > > > > while we deliberately do not fold int << 34 since the result is undefined there is > > IMHO no reason to not fold the above for any (even non-constant) shift value. > > We have guards with TYPE_OVERFLOW_SANITIZED in some cases but I think > > that's not appropriate here, there's one flag_sanitize check, maybe there's a > > special bit for SHIFT overflow we can use. Why is (X << 0) != 0 excempt in the > > condition? > > In this case, I think it makes more sense to err on the side of caution, and > avoid changing the observable behaviour of programs, even in cases were > the behaviour is officially undefined. For many targets, (1< always true for any value of x, but a counter example are x86's SSE shifts, > where shifts beyond the size of the vector result in zero. With STV, this > means that (1<<258) != 0 has a different value if performed as scalar vs. > performed as vector. Worse, one may end up with examples, where based > upon optimization level, we see different results as shift operands become > propagated constants in some paths, but was variable shifts others. > > Hence my personal preference is "first, do no harm" and limit this > transformation to the safe 0 <= X < MODE_PRECISION (mode). > Then given we'd like to avoid negative shifts, and therefore need to > test against zero, my second preference is "0 < X" over "0 <= X". > If the RTL contains a shift by zero, something strange is already going on > (these should be caught optimized elsewhere), and it's better to leave > these issues visible in the RTL, than paper over any "latent" mistakes. > > I fully I agree that this optimization could be more aggressive, but that > isn't required to resolve this PR, and resolving PR64992 only to open > the door for follow-up "unexpected behavior" PRs isn't great progress. > > Thoughts? Ok for mainline? OK - can you add a comment reflecting the above? An improvement might be to allow non-constant operands but test sth like expr_in_range (@1, 1, TYPE_PRECISION (...)) we already have expr_not_equal_to, expr_in_range could be done with value_range vr; if (get_global_range_query ()->range_of_expr (vr, @0) && vr.kind () == VR_RANGE) { wide_int wmin0 = vr.lower_bound (); wide_int wmax0 = vr.upper_bound (); ... there's a bunch of range_of_expr uses, I didn't check closely but some might fit a new expr_in_range utility. That can be done as followup (if you like). Btw, I wonder if bit-CCP would have not simplified the shift-by-constant compared to zero as well? If so, does that behave the same with respect to 0 or out of bound shifts? Richard. > > > > 2022-08-08 Roger Sayle > > > > > > gcc/ChangeLog > > > PR tree-optimization/64992 > > > PR tree-optimization/98956 > > > * match.pd (ne (lshift @0 @1) 0): Simplify (X << C) != 0 to X > > > when X is zero_one_valued_p and the shift constant C is valid. > > > (eq (lshift @0 @1) 0): Likewise, simplify (X << C) == 0 to !X > > > when X is zero_one_valued_p and the shift constant C is valid. > > > > > > gcc/testsuite/ChangeLog > > > PR tree-optimization/64992 > > > * gcc.dg/pr64992.c: New test case. > > > > > Thanks, > Roger > -- > >