From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 51C43385802D for ; Fri, 15 Oct 2021 11:30:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 51C43385802D Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 120C421A5F; Fri, 15 Oct 2021 11:30:31 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id D0F93A3B91; Fri, 15 Oct 2021 11:30:30 +0000 (UTC) Date: Fri, 15 Oct 2021 13:30:30 +0200 (CEST) From: Richard Biener To: Tamar Christina cc: gcc-patches@gcc.gnu.org, Jakub Jelinek , nd@arm.com Subject: Re: [PATCH] middle-end: fix de-optimizations with bitclear patterns on signed values In-Reply-To: Message-ID: References: MIME-Version: 1.0 X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: Fri, 15 Oct 2021 11:30:33 -0000 On Fri, 15 Oct 2021, Tamar Christina wrote: > Hi All, > > During testing after rebasing to commit I noticed a failing testcase with the > bitmask compare patch. > > Consider the following C++ testcase: > > #include > > #define A __attribute__((noipa)) > A bool f5 (double i, double j) { auto c = i <=> j; return c >= 0; } > > This turns into a comparison against chars, on systems where chars are signed > the pattern inserts an unsigned convert such that it's able to do the > transformation. > > i.e.: > > # RANGE [-1, 2] > # c$_M_value_22 = PHI <-1(3), 0(2), 2(5), 1(4)> > # RANGE ~[3, 254] > _11 = (unsigned char) c$_M_value_22; > _19 = _11 <= 1; > # .MEM_24 = VDEF <.MEM_6(D)> > D.10434 ={v} {CLOBBER}; > # .MEM_14 = VDEF <.MEM_24> > D.10407 ={v} {CLOBBER}; > # VUSE <.MEM_14> > return _19; > > instead of: > > # RANGE [-1, 2] > # c$_M_value_5 = PHI <-1(3), 0(2), 2(5), 1(4)> > # RANGE [-2, 2] > _3 = c$_M_value_5 & -2; > _19 = _3 == 0; > # .MEM_24 = VDEF <.MEM_6(D)> > D.10440 ={v} {CLOBBER}; > # .MEM_14 = VDEF <.MEM_24> > D.10413 ={v} {CLOBBER}; > # VUSE <.MEM_14> > return _19; > > This causes much worse codegen under -ffast-math due to phiops no longer > recognizing the pattern. It turns out that phiopts spaceship_replacement is > looking for the exact form that was just changed. > > Trying to get it to recognize the new form is not trivial as the transformation > doesn't look to work when the thing it's pointing to is itself a phi-node. What do you mean? Where it handles the BIT_AND it could also handle the conversion, no? The later handling would probably more explicitely need to distinguish between the BIT_AND and the conversion forms. Jakub? > Because of these issues this change delays the replacements until after loop > opts. This fixes the failing C++ testcase. > > Bootstrapped Regtested on aarch64-none-linux-gnu, > x86_64-pc-linux-gnu and no regressions. > > Ok for master? > > Thanks, > Tamar > > gcc/ChangeLog: > > * match.pd: Delay bitmask compare pattern till after loop opts. > > --- inline copy of patch -- > diff --git a/gcc/match.pd b/gcc/match.pd > index 9532cae582e152cae6e22fcce95a9744a844e3c2..d26e498447fc25a327a42cc6a119c6153d09ba03 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -4945,7 +4945,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > icmp (le le gt le gt) > (simplify > (cmp (bit_and:c@2 @0 cst@1) integer_zerop) > - (with { tree csts = bitmask_inv_cst_vector_p (@1); } > + (if (canonicalize_math_after_vectorization_p ()) > + (with { tree csts = bitmask_inv_cst_vector_p (@1); } > (switch > (if (csts && TYPE_UNSIGNED (TREE_TYPE (@1)) > && (VECTOR_TYPE_P (TREE_TYPE (@1)) || single_use (@2))) > @@ -4954,7 +4955,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > && (cmp == EQ_EXPR || cmp == NE_EXPR) > && (VECTOR_TYPE_P (TREE_TYPE (@1)) || single_use (@2))) > (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); } > - (icmp (convert:utype @0) { csts; })))))))) > + (icmp (convert:utype @0) { csts; }))))))))) > > /* -A CMP -B -> B CMP A. */ > (for cmp (tcc_comparison) > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)