From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x135.google.com (mail-lf1-x135.google.com [IPv6:2a00:1450:4864:20::135]) by sourceware.org (Postfix) with ESMTPS id 7BBCA3858017 for ; Fri, 14 Jul 2023 10:03:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7BBCA3858017 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-lf1-x135.google.com with SMTP id 2adb3069b0e04-4fbaef9871cso2926174e87.0 for ; Fri, 14 Jul 2023 03:03:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689329028; x=1691921028; 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=TSBUMz99cBWZ+/FUmbESfD+Q/wV3K3ShP7H+mE8F3kM=; b=Gb37y45mRcpyvsLuEqfibPqVrbs5+52cdO4dJuTGE6TQrIJLiUImaAMsVNt2Mf3yiu JLb6309bX9Kza9G0YTSF68awV2wvUFpGWs94KKRyUKW0RkpSKDxYQVBEfFIW5B+mrvBi 4pDOBmcuKJkTLmTDhw2zDEDQHoK82nmJPHLnHzPAKiIc1POY1nJqG46K43AIUFt8OYU3 h+KuUO7XSyPrMdYn2uZjg6INTWnzu8VYQxahCr9hD1pl9xBxaySK1SQIYElhDuMRVQEf aEAgsqmqKtuOZhDyeEC9tZc0busEO8eCopFizGLE1x22qLIDACbon/Q4Ad9ZNkmp1YiN i8pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689329028; x=1691921028; 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=TSBUMz99cBWZ+/FUmbESfD+Q/wV3K3ShP7H+mE8F3kM=; b=gj+9vgRJPhFov4xHDSPRtG1ZaFdn03yNLwwxFKOTMPSA/LF2b23/esdAzoOEyArA4r K7XwkXqkm87VNQimBKuOCF31Jui8gkEilpOA9GOuBzRo4V42lxMpsgK+HXzy2XGx0F2r eDgf7y3aqw7jP+a5hcbF8yUWBadSlb4YIouoZGjQLFjvmkwsVM82PyYWXV3KRzxYM05T dtbHprpcAkbMsL9Ow/zzW997HCAHqqNLpoJvl6j0VwlMof1pYG57wtrmNAhot56iKSXA sxpRIsNveV3PV/KGqdvT36o+Y1bow1b5W55OS82fg3ML3a9RHqfFM5gc3XtCd+4BL9fg M0YQ== X-Gm-Message-State: ABy/qLYir/EkgdkKC5r1pGMjbt45SWiCxlFGSoGerdStVLVBX8GRz8lV 2XTDpwjfR2LC6dOnnP2RC42xoKr4NA0eXeIQNOqAsZgyTD4= X-Google-Smtp-Source: APBJJlHVd2uAf6vpftCw59Eylt0VaxmZT0DrkEeRz+XIQUMSYempHSwOByilMbSDgKvSRQDPfK46NFhsBV/dsydyj3I= X-Received: by 2002:a05:6512:4025:b0:4f8:598b:e62d with SMTP id br37-20020a056512402500b004f8598be62dmr3788245lfb.56.1689329027410; Fri, 14 Jul 2023 03:03:47 -0700 (PDT) MIME-Version: 1.0 References: <042301d9b5ac$f84e3e60$e8eabb20$@nextmovesoftware.com> <006501d9b635$58c50890$0a4f19b0$@nextmovesoftware.com> In-Reply-To: <006501d9b635$58c50890$0a4f19b0$@nextmovesoftware.com> From: Uros Bizjak Date: Fri, 14 Jul 2023 12:03:35 +0200 Message-ID: Subject: Re: [x86 PATCH] PR target/110588: Add *bt_setncqi_2 to generate btl To: Roger Sayle Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,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 Fri, Jul 14, 2023 at 11:27=E2=80=AFAM Roger Sayle wrote: > > > > From: Uros Bizjak > > Sent: 13 July 2023 19:21 > > > > On Thu, Jul 13, 2023 at 7:10=E2=80=AFPM Roger Sayle > > wrote: > > > > > > This patch resolves PR target/110588 to catch another case in combine > > > where the i386 backend should be generating a btl instruction. This > > > adds another define_insn_and_split to recognize the RTL representatio= n > > > for this case. > > > > > > I also noticed that two related define_insn_and_split weren't using > > > the preferred string style for single statement > > > preparation-statements, so I've reformatted these to be consistent in= style with > > the new one. > > > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > > > and make -k check, both with and without --target_board=3Dunix{-m32} > > > with no new failures. Ok for mainline? > > > > > > > > > 2023-07-13 Roger Sayle > > > > > > gcc/ChangeLog > > > PR target/110588 > > > * config/i386/i386.md (*bt_setcqi): Prefer string form > > > preparation statement over braces for a single statement. > > > (*bt_setncqi): Likewise. > > > (*bt_setncqi_2): New define_insn_and_split. > > > > > > gcc/testsuite/ChangeLog > > > PR target/110588 > > > * gcc.target/i386/pr110588.c: New test case. > > > > +;; Help combine recognize bt followed by setnc (PR target/110588) > > +(define_insn_and_split "*bt_setncqi_2" > > + [(set (match_operand:QI 0 "register_operand") (eq:QI > > + (zero_extract:SWI48 > > + (match_operand:SWI48 1 "register_operand") > > + (const_int 1) > > + (zero_extend:SI (match_operand:QI 2 "register_operand"))) > > + (const_int 0))) > > + (clobber (reg:CC FLAGS_REG))] > > + "TARGET_USE_BT && ix86_pre_reload_split ()" > > + "#" > > + "&& 1" > > + [(set (reg:CCC FLAGS_REG) > > + (compare:CCC > > + (zero_extract:SWI48 (match_dup 1) (const_int 1) (match_dup 2)= ) > > + (const_int 0))) > > + (set (match_dup 0) > > + (ne:QI (reg:CCC FLAGS_REG) (const_int 0)))] > > + "operands[2] =3D lowpart_subreg (SImode, operands[2], QImode);") > > > > I don't think the above transformation is 100% correct, mainly due to t= he use of > > paradoxical subreg. > > > > The combined instruction is operating with a zero_extended QImode regis= ter, so > > all bits of the register are well defined. You are splitting using para= doxical subreg, > > so you don't know what garbage is there in the highpart of the count re= gister. > > However, BTL/BTQ uses modulo 64 (or 32) of this register, so even with = a slightly > > invalid RTX, everything checks out. > > > > + "operands[2] =3D lowpart_subreg (SImode, operands[2], QImode);") > > > > You probably need mode instead of SImode here. > > The define_insn for *bt is: > > (define_insn "*bt" > [(set (reg:CCC FLAGS_REG) > (compare:CCC > (zero_extract:SWI48 > (match_operand:SWI48 0 "nonimmediate_operand" "r,m") > (const_int 1) > (match_operand:SI 1 "nonmemory_operand" "r,")) > (const_int 0)))] > > So isn't appropriate here. > > But now you've made me think about it, it's inconsistent that all of the = shifts > and rotates in i386.md standardize on QImode for shift counts, but the bi= t test > instructions use SImode? I think this explains where the paradoxical SUB= REGs > come from, and in theory any_extend from QImode to SImode here could/shou= ld > be handled/unnecessary. > > Is it worth investigating a follow-up patch to convert all ZERO_EXTRACTs = and > SIGN_EXTRACTs in i386.md to use QImode (instead of SImode)? IIRC, zero_extract was moved from modeless to a pattern with defined mode a while ago. Perhaps SImode is just because of these ancient times, and BT pattern was written that way to satisfy combine. I think it is definitely worth investigating; perhaps some BT-related pattern will become obsolete because of the change. Uros.