From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by sourceware.org (Postfix) with ESMTPS id C0B68395542F for ; Tue, 1 Jun 2021 14:21:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C0B68395542F Received: by mail-pj1-x1029.google.com with SMTP id b15-20020a17090a550fb029015dad75163dso2051751pji.0 for ; Tue, 01 Jun 2021 07:21:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=m2NX8tGzU87M5qFCHNk3h3HgNms1HZexJ1muqNbKakU=; b=htlrvCMvpSUQbsFERfevi5hZOfT6ngGKX62NDiWoPgg7KLMtSdflkGR1JaY2GzK7ax Z+qmQH6ODQDLgxT7kVoTtw6hwboQ+SKCmNsMj7LH5MoWBpaZZMffFqIAHtAf9DBxg2cl +DTPvZtyjURcbUzKnGvjYKXhtFEqJG9M/r6wWVPsDRESyXJ91NJlFW5w5/udgXFHNyWh 0J+yLwc7NaRZYdXX+mMUwOMuMT/NYN8+xlyWvPC3uCaK6ssPkeLjnZz/+f8crESTlWAR L11jZW9AfMNfWSWmhxEJSi/dUCiVipPGmi1ckEm12cRxazCEcsvwkw70UGAib9FZ/mKA j7TA== X-Gm-Message-State: AOAM532nhu9/AUg1bTruYg2MzNzdm42AA8Rguf9ORgiYMEgu4P6Q4+fJ R5zDMqzngd3AMsvRuV41QmU= X-Google-Smtp-Source: ABdhPJw3pz4BGJHKm0sDQYJYqMiKj19XfUFubEI3n37qF52e03tlYnpRgVbjEPSmg6iWTziYTHNnkA== X-Received: by 2002:a17:90a:1b8f:: with SMTP id w15mr25133744pjc.101.1622557287722; Tue, 01 Jun 2021 07:21:27 -0700 (PDT) Received: from [172.31.0.175] (c-98-202-48-222.hsd1.ut.comcast.net. [98.202.48.222]) by smtp.gmail.com with ESMTPSA id h76sm14904408pfe.161.2021.06.01.07.21.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Jun 2021 07:21:27 -0700 (PDT) Subject: Re: [PATCH v2] Add vec_const_duplicate optab and TARGET_GEN_MEMSET_SCRATCH_RTX To: "H.J. Lu" , Richard Biener Cc: "H.J. Lu via Gcc-patches" , Jeff Law , Bernd Edlinger , Richard Sandiford References: <459318f2-a9b8-f542-a29e-0ecbbc82b69a@tachyum.com> From: Jeff Law Message-ID: Date: Tue, 1 Jun 2021 08:21:25 -0600 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.10.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Jun 2021 14:21:30 -0000 On 6/1/2021 7:29 AM, H.J. Lu via Gcc-patches wrote: > On Tue, Jun 1, 2021 at 6:25 AM Richard Biener > wrote: >> On Tue, Jun 1, 2021 at 3:05 PM H.J. Lu wrote: >>> On Mon, May 31, 2021 at 11:54:53PM -0600, Jeff Law wrote: >>>> >>>> On 5/31/2021 11:50 PM, Richard Sandiford wrote: >>>>> "H.J. Lu via Gcc-patches" writes: >>>>>> On Mon, May 31, 2021 at 06:32:04AM -0700, H.J. Lu wrote: >>>>>>> On Mon, May 31, 2021 at 6:26 AM Richard Biener >>>>>>> wrote: >>>>>>>> On Mon, May 31, 2021 at 3:12 PM H.J. Lu wrote: >>>>>>>>> On Mon, May 31, 2021 at 5:46 AM Richard Biener >>>>>>>>> wrote: >>>>>>>>>> On Mon, May 31, 2021 at 2:09 PM H.J. Lu wrote: >>>>>>>>>>> On Wed, May 26, 2021 at 10:28:16AM +0200, Richard Biener wrote: >>>>>>>>>>>>>>> -- Target Hook: rtx TARGET_GEN_MEMSET_VALUE (rtx DATA, scalar_int_mode >>>>>>>>>>>>>>> MODE) >>>>>>>>>>>>>>> This function returns the RTL of a register containing >>>>>>>>>>>>>>> 'GET_MODE_SIZE (MODE)' consecutive copies of the unsigned char >>>>>>>>>>>>>>> value given in the RTL register DATA. For example, if MODE is 4 >>>>>>>>>>>>>>> bytes wide, return the RTL for 0x01010101*DATA. >>>>>>>>>>>>>> For this one I wonder if it should be an optab instead. Couldn't you >>>>>>>>>>>>>> use the existing vec_duplicate for this by using (paradoxical) subregs >>>>>>>>>>>>>> like (subreg:TI (vec_duplicate:VnQI (subreg:VnQI (reg:QI ...)))? >>>>>>>>>>>>> I tried. It doesn't even work on x86. See: >>>>>>>>>>>>> >>>>>>>>>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570661.html >>>>>>>>>>>> Not sure what I should read from there... >>>>>>>>>>>> >>>>>>>>>>>>> There are special cases to subreg HI, SI and DI modes of TI mode in >>>>>>>>>>>>> ix86_gen_memset_value_from_prev. simplify_gen_subreg doesn't >>>>>>>>>>>>> work here. Each backend may need its own special handling. >>>>>>>>>>>> OK, I guess I'm not (RTL) qualified enough to further review these parts, >>>>>>>>>>>> sorry. Since we're doing code generation the canonical way to communicate >>>>>>>>>>>> with backends should be optabs, not some set of disconnected target hooks. >>>>>>>>>>>> But as said, I probably don't know enough of RTL to see why it's the only way. >>>>>>>>>>>> >>>>>>>>>>>> Richard. >>>>>>>>>>> Here is the patch to add optabs instead. Does it look OK? >>>>>>>>>>> >>>>>>>>>>> Thanks. >>>>>>>>>>> >>>>>>>>>>> H.J. >>>>>>>>>>> --- >>>>>>>>>>> Add 2 optabs: >>>>>>>>>>> >>>>>>>>>>> 1. integer_extract: Extract lower bit value from the integer value in >>>>>>>>>>> TImode, OImode or XImode. >>>>>>>>>> That sounds very specific, esp. the restriction to {TI,OI,XI}mode. >>>>>>>>>> It also sounds like it matches (subreg:{TI,OI,XI} (...) 0). There are >>>>>>>>>> existing target hooks verifying subreg validity - why's that not a good >>>>>>>>>> fit here? ISTR you say gen_lowpart () doesn't work (or was it >>>>>>>>>> simplify_gen_subreg?), why's that so? >>>>>>>>> {TI,OI,XI}mode are storage only integer types. subreg doesn't work >>>>>>>>> well on them. I got >>>>>>>>> >>>>>>>>> [hjl@gnu-cfl-2 pieces]$ cat s2.i >>>>>>>>> extern void *ops; >>>>>>>>> >>>>>>>>> void >>>>>>>>> foo (int c) >>>>>>>>> { >>>>>>>>> __builtin_memset (ops, c, 34); >>>>>>>>> } >>>>>>>>> [hjl@gnu-cfl-2 pieces]$ make s2.s >>>>>>>>> /export/build/gnu/tools-build/gcc-gitlab-debug/build-x86_64-linux/gcc/xgcc >>>>>>>>> -B/export/build/gnu/tools-build/gcc-gitlab-debug/build-x86_64-linux/gcc/ >>>>>>>>> -O2 -march=haswell -S s2.i >>>>>>>>> during RTL pass: reload >>>>>>>>> s2.i: In function ‘foo’: >>>>>>>>> s2.i:7:1: internal compiler error: maximum number of generated reload >>>>>>>>> insns per insn achieved (90) >>>>>>>>> 7 | } >>>>>>>>> | ^ >>>>>>>>> 0x1050734 lra_constraints(bool) >>>>>>>>> /export/gnu/import/git/gitlab/x86-gcc/gcc/lra-constraints.c:5091 >>>>>>>>> 0x1039536 lra(_IO_FILE*) >>>>>>>>> /export/gnu/import/git/gitlab/x86-gcc/gcc/lra.c:2336 >>>>>>>>> 0xfe1140 do_reload >>>>>>>>> /export/gnu/import/git/gitlab/x86-gcc/gcc/ira.c:5822 >>>>>>>>> 0xfe162e execute >>>>>>>>> /export/gnu/import/git/gitlab/x86-gcc/gcc/ira.c:6008 >>>>>>>>> Please submit a full bug report, >>>>>>>>> with preprocessed source if appropriate. >>>>>>>>> Please include the complete backtrace with any bug report. >>>>>>>>> See for instructions. >>>>>>>>> make: *** [Makefile:32: s2.s] Error 1 >>>>>>>>> [hjl@gnu-cfl-2 pieces]$ >>>>>>>>> >>>>>>>>> due to >>>>>>>>> >>>>>>>>> (insn 12 11 0 (set (mem:HI (plus:DI (reg/f:DI 84) >>>>>>>>> (const_int 32 [0x20])) [0 MEM [(void >>>>>>>>> *)ops.0_1]+32 S2 A8]) >>>>>>>>> (subreg:HI (reg:OI 51 xmm15) 0)) "s2.i":6:3 -1 >>>>>>>>> (nil)) >>>>>>>>> >>>>>>>>> The new optab gives us >>>>>>>>> >>>>>>>>> (insn 12 11 13 2 (set (reg:TI 88) >>>>>>>>> (reg:TI 51 xmm15)) "s2.i":6:3 -1 >>>>>>>>> (nil)) >>>>>>>>> (insn 13 12 14 2 (set (reg:SI 89) >>>>>>>>> (subreg:SI (reg:TI 88) 0)) "s2.i":6:3 -1 >>>>>>>>> (nil)) >>>>>>>>> (insn 14 13 15 2 (set (reg:HI 87) >>>>>>>>> (subreg:HI (reg:SI 89) 0)) "s2.i":6:3 -1 >>>>>>>>> (nil)) >>>>>>>> that looks odd to me - what's the final result after LRA? I think >>>>>>> I got: >>>>>>> >>>>>>> vmovd %edi, %xmm15 >>>>>>> movq ops(%rip), %rdx >>>>>>> vpbroadcastb %xmm15, %ymm15 >>>>>>> vmovq %xmm15, %rax <<<< move to GPR >>>>>>> vmovdqu %ymm15, (%rdx) >>>>>>> movw %ax, 32(%rdx) <<<< subreg of GPR >>>>>>> vzeroupper >>>>>>> ret >>>>>>> >>>>>>>> we should see to make lowpart_subreg work on {XI,OI,TI}mode. >>>>>>>> Only two steps should be necessary at most: >>>>>>>> xmm -> gpr, grp -> subreg, or gpr -> subreg. So the expander >>>>>>>> code in memset should try to generate the subreg directly >>>>>>> subreg didn't fail on x86 when I tried. >>>>>>> >>>>>>>> and if that fails, try a word_mode subreg followed by the subreg. >>>>>>> I will try word_mode subreg. >>>>>>> >>>>>> Here is the v2 patch to use word_mode subreg. For >>>>>> >>>>>> --- >>>>>> extern void *ops; >>>>>> >>>>>> void >>>>>> foo (int c) >>>>>> { >>>>>> __builtin_memset (ops, 4, 32); >>>>>> } >>>>>> --- >>>>>> >>>>>> without vec_const_duplicate, I got >>>>>> >>>>>> movl $4, %eax >>>>>> movq ops(%rip), %rdx >>>>>> movd %eax, %xmm0 >>>>>> punpcklbw %xmm0, %xmm0 >>>>>> punpcklwd %xmm0, %xmm0 >>>>>> pshufd $0, %xmm0, %xmm0 >>>>>> movups %xmm0, (%rdx) >>>>>> movups %xmm0, 16(%rdx) >>>>>> ret >>>>>> >>>>>> With vec_const_duplicate, I got >>>>>> >>>>>> movq ops(%rip), %rax >>>>>> movdqa .LC0(%rip), %xmm0 >>>>>> movups %xmm0, (%rax) >>>>>> movups %xmm0, 16(%rax) >>>>>> ret >>>>>> >>>>>> If vec_duplicate is allowed to fail, I don't need vec_const_duplicate. >>>>> I don't understand why we need an optab for this though. If the operand >>>>> is constant then we should just be doing an ordinary move in which the >>>>> source is a CONST_VECTOR. It's then up to the move patterns to handle >>>>> duplicated constants as efficiently as possible. (Sorry if this was >>>>> discussed upthread and I missed it.) >>>> That's exactly the point I'm trying to get across as well. >>>> >>> This is what we do today. But I'd like to generate >>> >>> movl $4, %eax >>> vpbroadcastb %eax, %ymm15 >>> movq ops(%rip), %rax >>> vmovdqu %ymm15, (%rax) >>> vzeroupper >>> ret >>> >>> instead of >>> >>> vmovdqa .LC0(%rip), %ymm15 >>> movq ops(%rip), %rax >>> vmovdqu %ymm15, (%rax) >>> vzeroupper >>> ret >>> >>> Do I need a vec_dup pattern for it? >> I think we have special code sequences to materialize some >> constant vectors already, we should be able to add to that, no? > We can do that for all 0s and all 1s at the final codegen. For > other values, since we need a GPR, we can't do that. You can catch them in your movxx expanders, you can create peep2 patterns that use available GPRs, etc.  I don't see a fundamental need to to introduce new target macros or hooks to handle this stuff.  In fact I've done both to handle a closely related issue on our port. jeff