From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com [IPv6:2607:f8b0:4864:20::b32]) by sourceware.org (Postfix) with ESMTPS id 3CFBC3858D1E for ; Thu, 13 Jul 2023 14:12:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3CFBC3858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu Received: by mail-yb1-xb32.google.com with SMTP id 3f1490d57ef6-bd61dd9a346so695041276.2 for ; Thu, 13 Jul 2023 07:12:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; t=1689257538; x=1691849538; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=oDmXF776Bfk3sznl42mB3LbxPU45HOwb3IrGml9XKN4=; b=i8KrBo+HDpjXfAVMNncSEC+HtMGanumF09vPlXK4cwUW/bRt2ZvHH1Fj3gBmxpL4yF SiWDvV35ft2u3qnFWBDJmobUdnHquhRYr+9Dja0AQh93+NlNd4qUHKSpojhEnWJmT9ai BZa3GZ3BsDizV+/WIV8etwZOSEaprvRNecuIcDQA8cKkRCSK4KS3O1CGHl/RBgcJL+Ux ODMZc0eRUxt6xegd66F5FYPXPTxWVkVllyFnhFqY6xy9fpB2sO6uSJ6Nm74BL+62HB3s Yp3awE6y7jXSyg9Pcjoh6rIzUR5bV0iOeNH3bpwkVoKQBEUASgRritsIn7Mqh6VeVps9 qfBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689257538; x=1691849538; h=content-transfer-encoding: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=oDmXF776Bfk3sznl42mB3LbxPU45HOwb3IrGml9XKN4=; b=CI+CZJFp5dst7Qpr/RTkBjZSJeqIApV6WHurCegYApgnw37/rI+Yfm1hxh4WeE7G3U pcKQU7+6syxraxlAqzP4CXRAsxWKtmAJBS/1ve8Y7mM+hr5NJeu/ft0HmMq4DIsi2N1z paBh9y9nyTsE1O/Z9MIF6C1NTlUv1Ngve+czZnxGgbjV/t/zMU9GTe1A7XU22SlAzvp8 6OBVrus+mIZ/ljw5hi1eSdeS4y+rrzPriHJOj0/Qv46lZrEyJ+7zfU9WTgA20uRkniZG YzGkstZqEUmHXvHF0BMKQYxacTPvHINaTK8UQaPUEdPaDuPlF0K9bg7D92E0fzWyA6fz DfYA== X-Gm-Message-State: ABy/qLaJXbfhz4Jcdh5ADy/TNBP58G/35rU1ETxrbr21LNP/g5CSHvYT iCeKu5z+RJUFQ5umEFW2Mj9ioZMVgeV9qpT2C1nUbQ== X-Google-Smtp-Source: APBJJlG+2Z51w5cOh4LWGIqgmgKD12ClTtw0G3OQLrQ3/xTCCR3G5ZPToWnQWzp0tvN5inx+8Ox8YADlO76qMpbvAM8= X-Received: by 2002:a0d:e250:0:b0:576:f783:cc9b with SMTP id l77-20020a0de250000000b00576f783cc9bmr1814728ywe.50.1689257538297; Thu, 13 Jul 2023 07:12:18 -0700 (PDT) MIME-Version: 1.0 References: <20230701092413.2488806-1-manolis.tsamis@vrull.eu> <20230701092413.2488806-3-manolis.tsamis@vrull.eu> In-Reply-To: From: Manolis Tsamis Date: Thu, 13 Jul 2023 17:11:41 +0300 Message-ID: Subject: Re: [PATCH 2/2] ifcvt: Allow more operations in multiple set if conversion To: Robin Dapp Cc: gcc-patches@gcc.gnu.org, Jakub Jelinek , Philipp Tomsich , Andrew Pinski Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,JMQ_SPF_NEUTRAL,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: I resent this with just the change in the comment. OK to merge? Manolis On Tue, Jul 4, 2023 at 5:32=E2=80=AFPM Manolis Tsamis wrote: > > On Mon, Jul 3, 2023 at 12:12=E2=80=AFPM Robin Dapp = wrote: > > > > Hi Manolis, > > > > that looks like a nice enhancement of what's already possible. The con= cern > > I had some years back already was that this function would eventually > > grow and cannibalize on some of what the other functions in ifcvt alrea= dy > > do :) At some point we really should unify but that's not within the > > scope of this patch. > > > > Hi Robin, > > Indeed and it would be nice to extend the multi statement > implementation to the point that the others are not needed :) > I have some future plans to analyze cases where the multi-statement > performs worse and improve on that. > > > IMHO we're already pretty far towards general "conditional execution" > > with conditional increments, selects and so on (and the function is sti= ll > > called "_noce") and historically the cond_exec functions would have > > taken care of that. To my knowledge though, none of the major backends > > implements anything like (cond_exec ...) anymore and relies on bit-twid= dling > > tricks to generate the conditional instructions. > > > > Have you checked whether cond_exec and others could be adjusted to > > handle the conditional instructions you want to see? They don't perfor= m > > full cost comparison though but just count. > > > > Thanks for mentioning that, I was not really aware of cond_exec usage. > As you say, it looks like cond_exec isn't used very much on major backend= s. > > Since noce_convert_multiple_sets_1 is just using the existing ifcvt > machinery (specifically noce_emit_cmove / try_emit_cmove_seq), is this > a question of whether we want to replace (if_then_else ...) with > (cond_exec ...) in general? > If that is beneficial then I could try to implement a change like > this, but that should probably be a separate effort from this > implementation. > > > I would expect a bit of discussion around that but from a first look > > I don't have major concerns. > > > > > -/* 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), an= d > > > - 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 o= ne > > > > Might want to change the "conditional moves" while you're at it. > > > > Thanks for pointing out this comment, I missed it. I will rewrite the > relevant parts. > > > > > > > - 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 functi= on 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)) > > > > We're potentially checking many more patterns than before. Maybe it > > would make sense to ask the backend whether it has a pattern for > > the respective code? > > > > Is it an issue if the backend doesn't have a pattern for a respective cod= e? > > My goal here is to not limit if conversion for sequences based on the > code but rather let ifcvt / the backedn decide based on costing. > That's because from what I've seen, conditional set instructions can > be beneficial even when the backend doesn't have a specific > instruction for that code. > > Best, > Manolis > > > Regards > > Robin > >