From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x634.google.com (mail-ej1-x634.google.com [IPv6:2a00:1450:4864:20::634]) by sourceware.org (Postfix) with ESMTPS id 0BAFE3896C3A for ; Wed, 16 Nov 2022 03:04:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0BAFE3896C3A 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-ej1-x634.google.com with SMTP id m22so40877655eji.10 for ; Tue, 15 Nov 2022 19:04:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=L4+m8SnmyWaIcGxupYsbtUlq6By+pZDdvtfkSjAiBSk=; b=Z8vQki6vHwdR3oEESGuWF++SlTkHowZQI9XC+ti4fZZJJDWyVSAFlHXU8hNtt7d0FG eMOzMBRGLnA9SJj1w2OwliGaFoIVUjQWpb/8JaGkrXsGgr0fLbmOUsvLhGikf3l98CGP FIkfNgNnq8qMYfnkG/nShV3Mh09cNVNF//S9gzmUn/jm4Qe+C94rwyJaOFFXmLqKcoGv ORD0sFUPj/OqKzocsPrq+iwomW7NUDhHmSMXHbzdMhvWBCH3VOrHERSYeUKrX7ZzBfbx GFRhtoWpwf7e3RnvcODm6IsigPA21o83+Rk/dJzNiHiAsPtZUB36Vd6k4uPeKdA1+eED e9Ag== 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:subject:date:message-id :reply-to; bh=L4+m8SnmyWaIcGxupYsbtUlq6By+pZDdvtfkSjAiBSk=; b=Z8fP6/RcEqOrlfMjtxFMHQxApxeEvYeL3fQR4hksF5k4Q+wOJTnyYqTC8wavMRyFxs TrDfMto9MoFxES+VIfHfbL8L2VCsJRmR1+qIEVsK78lgjCvUlSEfqvVK1SljeReh2wBD 3Ri0KC03pTYRjzktdzgRHcckNnumHvHitvWd8fQByTjJbcXtUV+MOGTf81VYYF2giTja 0qIJni/9gBkktqtCIOPO+h694xiVvlKHa3ZOowf5nAHF8jJHY4+5kwDaicnxv+4uFbq5 jWZpkKrraixj1b10NNeaGSWpyegx34lOtJF1SKJPEFO0ca5YgE3m2XzagEPQIxFcsFvo A8FQ== X-Gm-Message-State: ANoB5plsbeZC8ylQH2jYFVN5GztGnYOwXjFbBmAPofTF5hH1FslIGLyG AYNLtAnH7x+8ZL1xrEtfH0cloGlgiRqaueCBzFo= X-Google-Smtp-Source: AA0mqf5sQcGLQ12I/ExYyAWA25H2JtvGvqsMXkBqbZfv2z/CUpvaqxSot3edNddl2+2s8Tk0maguDZ/2M1T0x0vweWA= X-Received: by 2002:a17:906:4e0d:b0:7ad:b822:d2e4 with SMTP id z13-20020a1709064e0d00b007adb822d2e4mr16059277eju.35.1668567858256; Tue, 15 Nov 2022 19:04:18 -0800 (PST) MIME-Version: 1.0 References: <153badc6-8afc-0695-32b2-ab5a9e0a161d@linux.ibm.com> In-Reply-To: <153badc6-8afc-0695-32b2-ab5a9e0a161d@linux.ibm.com> From: David Edelsohn Date: Tue, 15 Nov 2022 22:04:04 -0500 Message-ID: Subject: Re: [rs6000, patch] Enable have_cbranchcc4 on rs6000 To: HAO CHEN GUI Cc: gcc-patches , Segher Boessenkool , "Kewen.Lin" , Peter Bergner Content-Type: multipart/alternative; boundary="00000000000079a0a505ed8dba02" X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,HTML_MESSAGE,KAM_NUMSUBJECT,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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: --00000000000079a0a505ed8dba02 Content-Type: text/plain; charset="UTF-8" On Tue, Nov 15, 2022 at 9:32 PM HAO CHEN GUI wrote: > Hi, > The patch enables have_cbrnachcc4 which is a flag in ifcvt.cc to > indicate if branch by CC bits is invalid or not. As rs6000 already has > "*cbranch" insn which does branching according to CC bits, the flag > should be enabled and relevant branches can be optimized out. The test > case illustrates the optimization. > > "*cbranch" is an anonymous insn which can't be generated directly. > So changing "const_int 0" to the third operand predicated by > "zero_constant" won't cause ICEs as orginal patterns still can be matched. > > Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. > Is this okay for trunk? Any recommendations? Thanks a lot. > > > ChangeLog > 2022-11-16 Haochen Gui > > gcc/ > * config/rs6000/rs6000.md (*cbranch): Rename to... > (cbranchcc4): ...this, and set const_int 0 to the third operand. > > gcc/testsuite/ > * gcc.target/powerpc/cbranchcc4.c: New. > > > patch.diff > diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md > index e9e5cd1e54d..ee171f21f6a 100644 > --- a/gcc/config/rs6000/rs6000.md > +++ b/gcc/config/rs6000/rs6000.md > @@ -13067,11 +13067,11 @@ (define_insn_and_split "*_cc" > ;; Conditional branches. > ;; These either are a single bc insn, or a bc around a b. > > -(define_insn "*cbranch" > +(define_insn "cbranchcc4" > [(set (pc) > (if_then_else (match_operator 1 "branch_comparison_operator" > [(match_operand 2 "cc_reg_operand" > "y") > - (const_int 0)]) > + (match_operand 3 "zero_constant")]) > (label_ref (match_operand 0)) > (pc)))] > "" > Shouldn't cbranchcc4 be a separate pattern? This pattern has an existing purpose and an expected ordering of operands. cbranchcc4 is passed the comparison operator as operand 0. Other ports ignore the second comparison operator and use (const_int 0). Operand 3 is the label, which seems to be the reason that you needed to change it to match_operand 3. It's great to add cbranchcc4 to the Power port where it definitely was an omission, but adapting *cbranch for that purpose is the wrong approach. The changes to the pattern are incorrect because they are covering up a difference in ordering of the operands. One can argue that the named pattern only enables the functionality in ifcvt and the pattern otherwise is used in its previous role. But this is a Frankenstein monster approach. You're trying to twist the existing pattern so that it triggers as cbranchcc4, but creating a pattern that messes up its arguments and only works because the new, named pattern never is called. This is too ugly. Please fix. Thanks, David diff --git a/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c > b/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c > new file mode 100644 > index 00000000000..1751d274bbf > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c > @@ -0,0 +1,8 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-rtl-ce1" } */ > +/* { dg-final {scan-rtl-dump "noce_try_store_flag_constants" "ce1" } } */ > + > +int test (unsigned int a, unsigned int b) > +{ > + return (a < b ? 0 : (a > b ? 2 : 1)); > +} > --00000000000079a0a505ed8dba02--