From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com [IPv6:2607:f8b0:4864:20::1036]) by sourceware.org (Postfix) with ESMTPS id 7075C3858D28 for ; Tue, 2 May 2023 21:08:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7075C3858D28 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-pj1-x1036.google.com with SMTP id 98e67ed59e1d1-24df161f84bso2299499a91.3 for ; Tue, 02 May 2023 14:08:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683061709; x=1685653709; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=053s5Iy1KewvXZ87Vc5cJp8vfjeCxdIqxFyFP5cie4A=; b=GBh/b0PAe5QJzYg1sWAxxGDOR+6V51l87hQxWqFAzsvcxatRd/YCFD6hfk9e4Xd5ru yXcbEtyb4PXnX+TI4PgrxksRnw7IS7RyqFe2tel59fgVaGniVYo3F8js26Xi8WNAYNlk z6Sm+wP+CVBK8oizk28AUxDlaKN1SGVnDzFRXwjm76yoQfZ+3ZNDxbpsXBxlTqyNYjUp +3MGIfuH8TKcteJW6e1S0Q+9bEgZQNK29JEpNJeKZeu7qvqWbZO5AOSto6FsAiCGiYNF wj1ZjI80lYD7+VJ9YG144UyhX8smObzodzox6cC0RUMSHZj60QRYld612T79f7JLhhFL kNag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683061709; x=1685653709; h=content-transfer-encoding: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=053s5Iy1KewvXZ87Vc5cJp8vfjeCxdIqxFyFP5cie4A=; b=bntY3NK4Q1y/GdnRwU0L9oIm4S2QuJ7S5SKI33J02GcZplHpLfyRV/AW+t7z8zP4dy KRo85STvOrx369YImlqJNy8zCJfmrElxSU7ZoPpDM5eAergghF52wdZOgLvHJxhzHXbX 9k04JuOKB5AGE8/83z96hn2aMCHlcIutiseL4X+TLXXRVJCH19GrsOeDQAYCCCepE7LB s8LLX/k1/6h0G4TdNVc070S3dpwIBIv8PMIq3KN2Mq0f+pPXeAwj1FvOIXOfZdGzb5So a774FkktkscUXwPpDm6mIOgs3UpKdHFRrjp4zI6s7H1lfR3ROJ6ER5tKLPj3Axe2cbue vIBg== X-Gm-Message-State: AC+VfDyUHTH6LPPFDj8Xz+hHDkhsU+c0lsSETxwJ6uw6HqTc4LZBKNfs gSPRXg4UwwYyIW87qy1G4dPve6LfRRAUc2dR8OA= X-Google-Smtp-Source: ACHHUZ4VKVIloInuV3ct9tbpFeAJqHUoRlPsuYm9A8noQR5V7kPIuqm04p4gJuCI/eXNw0dtuQ3M/K70IBwb/mIgg64= X-Received: by 2002:a17:90a:9114:b0:24e:35f6:981 with SMTP id k20-20020a17090a911400b0024e35f60981mr2072090pjo.38.1683061709238; Tue, 02 May 2023 14:08:29 -0700 (PDT) MIME-Version: 1.0 References: <20230428233446.688570-1-apinski@marvell.com> In-Reply-To: From: Andrew Pinski Date: Tue, 2 May 2023 14:08:16 -0700 Message-ID: Subject: Re: [PATCH] target: [PR109657] (a ? -1 : 0) | b could be optimized better for aarch64 To: Richard Sandiford , Andrew Pinski via Gcc-patches , Andrew Pinski Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_NUMSUBJECT,KAM_SHORT,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 Tue, May 2, 2023 at 5:23=E2=80=AFAM Richard Sandiford via Gcc-patches wrote: > > Andrew Pinski via Gcc-patches writes: > > There is no canonical form for this case defined. So the aarch64 backen= d needs > > a pattern to match both of these forms. > > > > The forms are: > > (set (reg/i:SI 0 x0) > > (if_then_else:SI (eq (reg:CC 66 cc) > > (const_int 0 [0])) > > (reg:SI 97) > > (const_int -1 [0xffffffffffffffff]))) > > and > > (set (reg/i:SI 0 x0) > > (ior:SI (neg:SI (ne:SI (reg:CC 66 cc) > > (const_int 0 [0]))) > > (reg:SI 102))) > > > > Currently the aarch64 backend matches the first form so this > > patch adds a insn_and_split to match the second form and > > convert it to the first form. > > > > OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions > > > > PR target/109657 > > > > gcc/ChangeLog: > > > > * config/aarch64/aarch64.md (*cmov_insn_m1): New > > insn_and_split pattern. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/aarch64/csinv-2.c: New test. > > --- > > gcc/config/aarch64/aarch64.md | 20 +++++++++++++++++ > > gcc/testsuite/gcc.target/aarch64/csinv-2.c | 26 ++++++++++++++++++++++ > > 2 files changed, 46 insertions(+) > > create mode 100644 gcc/testsuite/gcc.target/aarch64/csinv-2.c > > > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64= .md > > index e1a2b265b20..57fe5601350 100644 > > --- a/gcc/config/aarch64/aarch64.md > > +++ b/gcc/config/aarch64/aarch64.md > > @@ -4194,6 +4194,26 @@ (define_insn "*cmovsi_insn_uxtw" > > [(set_attr "type" "csel, csel, csel, csel, csel, mov_imm, mov_imm")] > > ) > > > > +;; There are two canonical forms for `cmp ? -1 : a`. > > +;; This is the second form and is here to help combine. > > +;; Support `-(cmp) | a` into `cmp ? -1 : a` to be canonical in the bac= kend. > > +(define_insn_and_split "*cmov_insn_m1" > > + [(set (match_operand:GPI 0 "register_operand" "=3Dr") > > + (ior:GPI > > + (neg:GPI > > + (match_operator:GPI 1 "aarch64_comparison_operator" > > + [(match_operand 2 "cc_register" "") (const_int 0)])) > > + (match_operand 3 "register_operand" "r")))] > > + "" > > + "#" > > + "&& true" > > + [(set (match_dup 0) > > + (if_then_else:GPI (match_dup 1) > > + (const_int -1) (match_dup 3)))] > > Sorry for the nit, but the formatting of the last two lines looks odd IMO= . > How about: > > (if_then_else:GPI (match_dup 1) (const_int -1) (match_dup 3))... > > or: > > (if_then_else:GPI (match_dup 1) > (const_int -1) > (match_dup 3))... > > OK with that change, thanks. I committed with the second form as it is easier to read than all on one line I think. Thanks, Andrew > > Richard > > > + {} > > + [(set_attr "type" "csel")] > > +) > > + > > (define_insn "*cmovdi_insn_uxtw" > > [(set (match_operand:DI 0 "register_operand" "=3Dr") > > (if_then_else:DI > > diff --git a/gcc/testsuite/gcc.target/aarch64/csinv-2.c b/gcc/testsuite= /gcc.target/aarch64/csinv-2.c > > new file mode 100644 > > index 00000000000..89132acb713 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/aarch64/csinv-2.c > > @@ -0,0 +1,26 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2" } */ > > +/* PR target/109657: (a ? -1 : 0) | b could be better */ > > + > > +/* Both functions should have the same assembly of: > > + cmp w1, 0 > > + csinv w0, w0, wzr, eq > > + > > + We should not get: > > + cmp w1, 0 > > + csetm w1, ne > > + orr w0, w1, w0 > > + */ > > +/* { dg-final { scan-assembler-times "csinv\tw\[0-9\]" 2 } } */ > > +/* { dg-final { scan-assembler-not "csetm\tw\[0-9\]" } } */ > > +unsigned b(unsigned a, unsigned b) > > +{ > > + if(b) > > + return -1; > > + return a; > > +} > > +unsigned b1(unsigned a, unsigned b) > > +{ > > + unsigned t =3D b ? -1 : 0; > > + return a | t; > > +}