From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52d.google.com (mail-pg1-x52d.google.com [IPv6:2607:f8b0:4864:20::52d]) by sourceware.org (Postfix) with ESMTPS id C316C384AB55 for ; Tue, 23 Apr 2024 11:01:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C316C384AB55 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C316C384AB55 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::52d ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713870100; cv=none; b=vGlMuZ7egtuho3ceDKXyzs3tzEhWexFfTZMQC5qaqh4f3BsQ4sXtFZQKYM5ujV6zN97rxPLrV0/v3n6mPqRZKfSFjVooKdDdr8ijt3LKzulpfEEEyLUcwyraTiNloaJZddMiLyJ1XNL8l5SOLzVRvY5SXIyyKLnmXoC2MQVE71c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713870100; c=relaxed/simple; bh=nn/Y9+Sr+BBBe8zRYxM3SGqdGLHCC667eV5ZPPqBHRE=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=C8+4RDqGh5gidxvBynHb/dzoe31Dw6cnjoknhs3cjW/wSu5fNYXaA+QVfaukm4eMuLoaoFMalxWYR5QAqi+VwtCejh0+SMZf3nvRYUZ2QqzgFtnkzqURdiejNodAYcEBBuqi31wu+HE7aAtahWkgCsbFAIxTp+nFr/atFb9nX3U= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x52d.google.com with SMTP id 41be03b00d2f7-5d8b887bb0cso4166363a12.2 for ; Tue, 23 Apr 2024 04:01:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; t=1713870088; x=1714474888; darn=gcc.gnu.org; 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=pjh5DGH6V0SOezkcLyJjfeQfvP5cFWq+dUYDQGL2Iss=; b=VBVvMQFmP+YEClRgiHETSRMjHVIN7k+VbSBFzbAXTfUc4g10JROJFZaA7vP8I3I1wa NYsvDhmCZ8fajUOxVrrLKHWTExNNnJTuqQRak36UPUI4J3OWerU0P4o2sWGVLAhXcb6W EVYUO0Nzu4bU3R36pf7SpdAI13dIlKD2KR8SYF7wqGtWMYd2qb52DZpSWrD80Dddl0UC jz6vaPz3XUiKtSLjyP2jMqV8IFoG6dSTXIAW7Hu+JoZeKncfj0qNvC0+DRtXVOpZ0Zlc jwPUdlaeAtwWs5/yj+ZjgMHyLMvUJcDxwdNXiAPM0Q8/bGHwdWoIfehPhrSVvCOI+j0Z J2SQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713870088; x=1714474888; 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=pjh5DGH6V0SOezkcLyJjfeQfvP5cFWq+dUYDQGL2Iss=; b=EqWF6QiHPABIFTmyBaUnw5YvUq4mAKE+KlSSK4cZUE1+gTuvVJXOZ0OVGLt4zF7Tpu fBP2AI2qZQEE80++6HP8U0MMH7vca5mIHPL77ZEdErgTm0cU8JKgMLOWh7/tvMAvNW3Z HbZwzI4v986LDP8QE6jSkXqncADGqjIPOzbvhD7mh7Ww7uDR0Ntx9qb9n8NURuavp49Y k80mbXBkUDxO70uJGf+mAbPPfT4G/atKWceIk8FmFmdV1Ppp/fW+v3nvmEcdKgwCcInc qZZno3CrJtgg9iqeW21TMnbRp0hnH5sEMhBauPHm0G3tXkQCKrY35d+5Kb2t1P94y5aK ycNQ== X-Forwarded-Encrypted: i=1; AJvYcCUUkzfrrYWEf5Bhtm6VKCheYAWcflG1I32NT3tr7qWKIc7KGcYBmtRpwGNLENm2syc6g7I+R6I/OWcBOOdW73euzkS4IFMwOg== X-Gm-Message-State: AOJu0YxJogGtFzuPA2DCzf47Kc2X6wA5UbYHjpomhDiik8nD/3Hti7SH 5T3YmNPynOpEP+Lf2spzueqYodvNT8LI9UUGQRwmOYVU0iF6wRFzsnC1PImQfxQMd4g5M1CBzVK qygJ6hD8qRJFJzJ6sOheJ5pOJo6K01XlPCgh5Fw== X-Google-Smtp-Source: AGHT+IGZnb4OoK6AxFoYjOjKyB1ltCnFbMxc6AEaswQxyjVSFx4F2MxHqDKCbO9ziljaEeUtvTFAvssX55+pcLSV/VI= X-Received: by 2002:a17:90a:4d86:b0:2a2:2fd:8bd8 with SMTP id m6-20020a17090a4d8600b002a202fd8bd8mr13197610pjh.15.1713870088383; Tue, 23 Apr 2024 04:01:28 -0700 (PDT) MIME-Version: 1.0 References: <20230830101400.1539313-1-manolis.tsamis@vrull.eu> <20230830101400.1539313-3-manolis.tsamis@vrull.eu> In-Reply-To: From: Manolis Tsamis Date: Tue, 23 Apr 2024 14:00:52 +0300 Message-ID: Subject: Re: [PATCH v3 2/4] ifcvt: Allow more operations in multiple set if conversion To: Manolis Tsamis , gcc-patches@gcc.gnu.org, Philipp Tomsich , Robin Dapp , Jakub Jelinek , richard.sandiford@arm.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,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: On Thu, Oct 19, 2023 at 10:46=E2=80=AFPM Richard Sandiford wrote: > > Manolis Tsamis writes: > > Currently the operations allowed for if conversion of a basic block wit= h > > multiple sets are few, namely REG, SUBREG and CONST_INT (as controlled = by > > bb_ok_for_noce_convert_multiple_sets). > > > > This commit allows more operations (arithmetic, compare, etc) to partic= ipate > > in if conversion. The target's profitability hook and ifcvt's costing i= s > > expected to reject sequences that are unprofitable. > > > > This is especially useful for targets which provide a rich selection of > > conditional instructions (like aarch64 which has cinc, csneg, csinv, cc= mp, ...) > > which are currently not used in basic blocks with more than a single se= t. > > > > gcc/ChangeLog: > > > > * ifcvt.cc (try_emit_cmove_seq): Modify comments. > > (noce_convert_multiple_sets_1): Modify comments. > > (bb_ok_for_noce_convert_multiple_sets): Allow more operations. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/aarch64/ifcvt_multiple_sets_arithm.c: New test. > > > > Signed-off-by: Manolis Tsamis > > --- > > > > Changes in v3: > > - Add SCALAR_INT_MODE_P check in bb_ok_for_noce_convert_multipl= e_sets. > > - Allow rewiring of multiple regs. > > - Refactor code with noce_multiple_sets_info. > > - Remove old code for subregs. > > > > gcc/ifcvt.cc | 63 ++++++++++----- > > .../aarch64/ifcvt_multiple_sets_arithm.c | 79 +++++++++++++++++++ > > 2 files changed, 123 insertions(+), 19 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/aarch64/ifcvt_multiple_set= s_arithm.c > > > > diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc > > index 3273aeca125..efe8ab1577a 100644 > > --- a/gcc/ifcvt.cc > > +++ b/gcc/ifcvt.cc > > @@ -3215,13 +3215,13 @@ try_emit_cmove_seq (struct noce_if_info *if_inf= o, rtx temp, > > /* We have something like: > > > > if (x > y) > > - { i =3D a; j =3D b; k =3D c; } > > + { i =3D EXPR_A; j =3D EXPR_B; k =3D EXPR_C; } > > > > Make it: > > > > - tmp_i =3D (x > y) ? a : i; > > - tmp_j =3D (x > y) ? b : j; > > - tmp_k =3D (x > y) ? c : k; > > + tmp_i =3D (x > y) ? EXPR_A : i; > > + tmp_j =3D (x > y) ? EXPR_B : j; > > + tmp_k =3D (x > y) ? EXPR_C : k; > > i =3D tmp_i; > > j =3D tmp_j; > > k =3D tmp_k; > > @@ -3637,11 +3637,10 @@ noce_convert_multiple_sets_1 (struct noce_if_in= fo *if_info, > > > > > > > > -/* Return true iff basic block TEST_BB is comprised of only > > - (SET (REG) (REG)) insns suitable for conversion to a series > > - of conditional moves. Also check that we have more than one set > > - (other routines can handle a single set better than we would), and > > - fewer than PARAM_MAX_RTL_IF_CONVERSION_INSNS sets. While going > > +/* Return true iff basic block TEST_BB is suitable for conversion to a > > + series of conditional moves. Also check that we have more than one > > + set (other routines can handle a single set better than we would), > > + and fewer than PARAM_MAX_RTL_IF_CONVERSION_INSNS sets. While going > > through the insns store the sum of their potential costs in COST. = */ > > > > static bool > > @@ -3667,20 +3666,46 @@ bb_ok_for_noce_convert_multiple_sets (basic_blo= ck test_bb, unsigned *cost) > > rtx dest =3D SET_DEST (set); > > rtx src =3D SET_SRC (set); > > > > - /* We can possibly relax this, but for now only handle REG to RE= G > > - (including subreg) moves. This avoids any issues that might com= e > > - from introducing loads/stores that might violate data-race-freed= om > > - guarantees. */ > > - if (!REG_P (dest)) > > + /* Do not handle anything involving memory loads/stores since it= might > > + violate data-race-freedom guarantees. */ > > + if (!REG_P (dest) || contains_mem_rtx_p (src)) > > + return false; > > + > > + if (!SCALAR_INT_MODE_P (GET_MODE (src))) > > return false; > > > > - if (!((REG_P (src) || CONSTANT_P (src)) > > - || (GET_CODE (src) =3D=3D SUBREG && REG_P (SUBREG_REG (src)) > > - && subreg_lowpart_p (src)))) > > + /* Allow a wide range of operations and let the costing function= decide > > + if the conversion is worth it later. */ > > + enum rtx_code code =3D GET_CODE (src); > > + if (!(CONSTANT_P (src) > > + || code =3D=3D REG > > + || code =3D=3D SUBREG > > + || code =3D=3D ZERO_EXTEND > > + || code =3D=3D SIGN_EXTEND > > + || code =3D=3D NOT > > + || code =3D=3D NEG > > + || code =3D=3D PLUS > > + || code =3D=3D MINUS > > + || code =3D=3D AND > > + || code =3D=3D IOR > > + || code =3D=3D MULT > > + || code =3D=3D ASHIFT > > + || code =3D=3D ASHIFTRT > > + || code =3D=3D NE > > + || code =3D=3D EQ > > + || code =3D=3D GE > > + || code =3D=3D GT > > + || code =3D=3D LE > > + || code =3D=3D LT > > + || code =3D=3D GEU > > + || code =3D=3D GTU > > + || code =3D=3D LEU > > + || code =3D=3D LTU > > + || code =3D=3D COMPARE)) > > return false; > > I'm nervous about lists of operations like these, for two reasons: > > (1) It isn't obvious what criteria are used to select the codes. > > (2) It requires the top-level code to belong to a given set, but it > allows subrtxes of src to be arbitrarily complex. E.g. (to pick > a silly example) a toplevel (popcount ...) would be rejected, but > (plus (popcount ...) (const_int 1)) would be OK. > > Could we just remove this condition instead? > True and done: https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649899.= html I also removed the if (!SCALAR_INT_MODE_P (GET_MODE (src))) because it was rejecting (const_int) apparently. The mode is checked in the rest of the code so it wasn't needed in the first place. Manolis > Thanks, > Richard > > > > > - /* Destination must be appropriate for a conditional write. */ > > - if (!noce_operand_ok (dest)) > > + /* Destination and source must be appropriate. */ > > + if (!noce_operand_ok (dest) || !noce_operand_ok (src)) > > return false; > > > > /* We must be able to conditionally move in this mode. */ > > diff --git a/gcc/testsuite/gcc.target/aarch64/ifcvt_multiple_sets_arith= m.c b/gcc/testsuite/gcc.target/aarch64/ifcvt_multiple_sets_arithm.c > > new file mode 100644 > > index 00000000000..d977f4d62ec > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/aarch64/ifcvt_multiple_sets_arithm.c > > @@ -0,0 +1,79 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-rtl-ce1" } */ > > + > > +void sink2(int, int); > > +void sink3(int, int, int); > > + > > +void cond1(int cond, int x, int y) > > +{ > > + if (cond) > > + { > > + x =3D x << 4; > > + y =3D 1; > > + } > > + > > + sink2(x, y); > > +} > > + > > +void cond2(int cond, int x, int y) > > +{ > > + if (cond) > > + { > > + x++; > > + y++; > > + } > > + > > + sink2(x, y); > > +} > > + > > +void cond3(int cond, int x1, int x2, int x3) > > +{ > > + if (cond) > > + { > > + x1++; > > + x2++; > > + x3++; > > + } > > + > > + sink3(x1, x2, x3); > > +} > > + > > +void cond4(int cond, int x, int y) > > +{ > > + if (cond) > > + { > > + x +=3D 2; > > + y +=3D 3; > > + } > > + > > + sink2(x, y); > > +} > > + > > +void cond5(int cond, int x, int y, int r1, int r2) > > +{ > > + if (cond) > > + { > > + x =3D r1 + 2; > > + y =3D r2 - 34; > > + } > > + > > + sink2(x, y); > > +} > > + > > +void cond6(int cond, int x, int y) > > +{ > > + if (cond) > > + { > > + x =3D -x; > > + y =3D ~y; > > + } > > + > > + sink2(x, y); > > +} > > + > > +/* { dg-final { scan-assembler-times "cinc\t" 5 } } */ > > +/* { dg-final { scan-assembler-times "csneg\t" 1 } } */ > > +/* { dg-final { scan-assembler-times "csinv\t" 1 } } */ > > +/* { dg-final { scan-assembler "csel\t" } } */ > > + > > +/* { dg-final { scan-rtl-dump-times "if-conversion succeeded through n= oce_convert_multiple_sets" 6 "ce1" } } */ > > \ No newline at end of file