From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by sourceware.org (Postfix) with ESMTPS id DB4643858004 for ; Mon, 31 May 2021 20:22:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DB4643858004 Received: by mail-pj1-x102e.google.com with SMTP id o17-20020a17090a9f91b029015cef5b3c50so172603pjp.4 for ; Mon, 31 May 2021 13:22:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=SkJJ3/7hm6aqnQHBJ0tVu5R9iTjNGvrw6V3Bq91xRMw=; b=qr3r4wtv0gxjYVHSJ0qugEmRXDhbKtVBV3RYfXyDWqIk0xzChagRXGULnYwc1hg6gQ V/NBqMWOCNVNWpf5lYLNAMGYXoM/gEq4W0r8JrCGLnQQxMdMV6sxHXqCr+uIC9M1rAGP BFExqxpqaFtSQJcGn4Kb+Od6J7uXH9txwgYtRErorgT7fwIZ1HtRgWmUiPIhLsg1EUq4 Qspz3N2vDIP1scHmklr0YzDr1bjVbWKuq3SXDwG7Bp17yjetc77wpHvJk5Yt4J+33vsv Rhi+AWRSrizU5HNcvey0selZkhIUx9y/HPwFswfR/RwTYPFYkSQwKAfPWkRT/Tcb7M5W xLXA== X-Gm-Message-State: AOAM5320TYuODBV9+8RVQQC0l52uDVL67lmZ0DxY26PmgHG5YUiTNPfF M3WfI03S5/xTEmjLBlmg/ZY= X-Google-Smtp-Source: ABdhPJxemNTQb+PUnMhGmzU7kba5Jnezqzd5qdxzCcBVUl/Xu/eqqXoxETDB+uLlLbmXXG4vZ7Qvfw== X-Received: by 2002:a17:90b:1d0f:: with SMTP id on15mr848847pjb.194.1622492554743; Mon, 31 May 2021 13:22:34 -0700 (PDT) Received: from gnu-cfl-2.localdomain ([172.56.38.102]) by smtp.gmail.com with ESMTPSA id i8sm11804486pgt.58.2021.05.31.13.22.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 31 May 2021 13:22:33 -0700 (PDT) Received: by gnu-cfl-2.localdomain (Postfix, from userid 1000) id 094EBC039C; Mon, 31 May 2021 13:22:32 -0700 (PDT) Date: Mon, 31 May 2021 13:22:32 -0700 From: "H.J. Lu" To: Richard Biener , GCC Patches , Richard Sandiford , Uros Bizjak , Bernd Edlinger Cc: Jeff Law Subject: [PATCH v2] Add vec_const_duplicate optab and TARGET_GEN_MEMSET_SCRATCH_RTX Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-3035.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, 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: Mon, 31 May 2021 20:22:38 -0000 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. H.J. --- 1. Add vec_const_duplicate to broadcast a QImode constant to a vector. It is similar to vec_duplicate which is not allowed to fail. Since the resulting vector is computable at compile-time, vec_duplicate may not be faster and backend can opt out broadcasting from a constant while opting in broadcasting from a variable. 2. Rewrite builtin_memset_read_str/builtin_memset_gen_str to support target instructions to duplicate QImode value to TImode, OImode or XImode value for memmset. 3. Add TARGET_GEN_MEMSET_SCRATCH_RTX to allow the backend to use a hard scratch register to avoid stack realignment when expanding memset. PR middle-end/90773 * builtins.c (gen_memset_value_from_prev): New function. (gen_memset_broadcast): Likewise. (builtin_memset_read_str): Use gen_memset_value_from_prev and gen_memset_broadcast. (builtin_memset_gen_str): Likewise. * optabs.def: Add vec_const_duplicate. * target.def (gen_memset_scratch_rtx): New hook. * doc/md.texi: Document vec_const_duplicate. * doc/tm.texi.in: Add TARGET_GEN_MEMSET_SCRATCH_RTX. * doc/tm.texi: Regenerated. --- gcc/builtins.c | 115 +++++++++++++++++++++++++++++++++++++-------- gcc/doc/md.texi | 10 ++++ gcc/doc/tm.texi | 5 ++ gcc/doc/tm.texi.in | 2 + gcc/optabs.def | 2 + gcc/target.def | 7 +++ 6 files changed, 121 insertions(+), 20 deletions(-) diff --git a/gcc/builtins.c b/gcc/builtins.c index af1fe49bb48..4573450d2c0 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -6598,26 +6598,104 @@ expand_builtin_strncpy (tree exp, rtx target) return NULL_RTX; } +/* Return the RTL of a register in MODE generated from PREV in the + previous iteration. */ + +static rtx +gen_memset_value_from_prev (void *prevp, scalar_int_mode mode) +{ + rtx target = nullptr; + by_pieces_prev *prev = (by_pieces_prev *) prevp; + if (prev != nullptr && prev->data != nullptr) + { + /* Use the previous data in the same mode. */ + if (prev->mode == mode) + return prev->data; + + rtx prev_rtx = prev->data; + machine_mode prev_mode = prev->mode; + unsigned int word_size = GET_MODE_SIZE (word_mode); + if (word_size < GET_MODE_SIZE (prev->mode) + && word_size > GET_MODE_SIZE (mode)) + { + /* First generate subreg of word mode if the previous mode is + wider than word mode and word mode is wider than MODE. */ + prev_rtx = simplify_gen_subreg (word_mode, prev_rtx, + prev_mode, 0); + prev_mode = word_mode; + } + if (prev_rtx != nullptr) + target = simplify_gen_subreg (mode, prev_rtx, prev_mode, 0); + } + return target; +} + +/* Return the RTL of a register in MODE broadcasted from DATA. */ + +static rtx +gen_memset_broadcast (enum optab_tag broadcast_optab, rtx data, + scalar_int_mode mode) +{ + /* Skip if regno_reg_rtx isn't initialized. */ + if (!regno_reg_rtx) + return nullptr; + + rtx target = nullptr; + + unsigned int nunits = GET_MODE_SIZE (mode) / GET_MODE_SIZE (QImode); + machine_mode vector_mode; + if (!mode_for_vector (QImode, nunits).exists (&vector_mode)) + gcc_unreachable (); + + enum insn_code icode = optab_handler (broadcast_optab, vector_mode); + if (icode != CODE_FOR_nothing) + { + target = targetm.gen_memset_scratch_rtx (vector_mode); + class expand_operand ops[2]; + create_output_operand (&ops[0], target, vector_mode); + create_input_operand (&ops[1], (rtx) data, QImode); + expand_insn (icode, 2, ops); + if (!rtx_equal_p (target, ops[0].value)) + emit_move_insn (target, ops[0].value); + if (REGNO (target) < FIRST_PSEUDO_REGISTER) + target = gen_rtx_REG (mode, REGNO (target)); + else + target = convert_to_mode (mode, target, 1); + } + + return target; +} + /* Callback routine for store_by_pieces. Read GET_MODE_BITSIZE (MODE) bytes from constant string DATA + OFFSET and return it as target constant. If PREV isn't nullptr, it has the RTL info from the previous iteration. */ rtx -builtin_memset_read_str (void *data, void *prevp, +builtin_memset_read_str (void *data, void *prev, HOST_WIDE_INT offset ATTRIBUTE_UNUSED, scalar_int_mode mode) { - by_pieces_prev *prev = (by_pieces_prev *) prevp; - if (prev != nullptr && prev->data != nullptr) + rtx target; + + /* Don't use the previous value if size is 1. */ + if (GET_MODE_SIZE (mode) != 1) { - /* Use the previous data in the same mode. */ - if (prev->mode == mode) - return prev->data; + target = gen_memset_value_from_prev (prev, mode); + if (target != nullptr) + return target; } const char *c = (const char *) data; - char *p = XALLOCAVEC (char, GET_MODE_SIZE (mode)); + char *p = XALLOCAVEC (char, GET_MODE_SIZE (QImode)); + memset (p, *c, GET_MODE_SIZE (QImode)); + rtx src = c_readstr (p, QImode); + target = gen_memset_broadcast (vec_const_duplicate_optab, src, + mode); + if (target != nullptr) + return target; + + p = XALLOCAVEC (char, GET_MODE_SIZE (mode)); memset (p, *c, GET_MODE_SIZE (mode)); @@ -6631,7 +6709,7 @@ builtin_memset_read_str (void *data, void *prevp, nullptr, it has the RTL info from the previous iteration. */ static rtx -builtin_memset_gen_str (void *data, void *prevp, +builtin_memset_gen_str (void *data, void *prev, HOST_WIDE_INT offset ATTRIBUTE_UNUSED, scalar_int_mode mode) { @@ -6639,22 +6717,19 @@ builtin_memset_gen_str (void *data, void *prevp, size_t size; char *p; - by_pieces_prev *prev = (by_pieces_prev *) prevp; - if (prev != nullptr && prev->data != nullptr) - { - /* Use the previous data in the same mode. */ - if (prev->mode == mode) - return prev->data; - - target = simplify_gen_subreg (mode, prev->data, prev->mode, 0); - if (target != nullptr) - return target; - } - size = GET_MODE_SIZE (mode); if (size == 1) return (rtx) data; + target = gen_memset_value_from_prev (prev, mode); + if (target != nullptr) + return target; + + target = gen_memset_broadcast (vec_duplicate_optab, (rtx) data, + mode); + if (target != nullptr) + return target; + p = XALLOCAVEC (char, size); memset (p, 1, size); coeff = c_readstr (p, mode); diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi index 00caf3844cc..cb9cf420c0b 100644 --- a/gcc/doc/md.texi +++ b/gcc/doc/md.texi @@ -5079,6 +5079,16 @@ vectors go through the @code{mov@var{m}} pattern instead. This pattern is not allowed to @code{FAIL}. +@cindex @code{vec_const_duplicate@var{m}} instruction pattern +@item @samp{vec_const_duplicate@var{m}} +Initialize vector output operand 0 in mode @var{m} so that each element +has the value given by constant input operand 1. + +This pattern only handles duplicates of constant inputs. Non-constant +vectors go through the @code{vec_duplicate@var{m}} pattern instead. + +This pattern is not allowed to @code{FAIL}. + @cindex @code{vec_series@var{m}} instruction pattern @item @samp{vec_series@var{m}} Initialize vector output operand 0 so that element @var{i} is equal to diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index e3a080e4a7c..8ccc262b1fc 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -11894,6 +11894,11 @@ This function prepares to emit a conditional comparison within a sequence @var{bit_code} is @code{AND} or @code{IOR}, which is the op on the compares. @end deftypefn +@deftypefn {Target Hook} rtx TARGET_GEN_MEMSET_SCRATCH_RTX (machine_mode @var{mode}) +This hook should return an rtx for scratch register in @var{mode} to +be used by memset broadcast. The default is @code{gen_reg_rtx}. +@end deftypefn + @deftypefn {Target Hook} unsigned TARGET_LOOP_UNROLL_ADJUST (unsigned @var{nunroll}, class loop *@var{loop}) This target hook returns a new value for the number of times @var{loop} should be unrolled. The parameter @var{nunroll} is the number of times diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index d9fbbe20e6f..99bf01fe25d 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -7960,6 +7960,8 @@ lists. @hook TARGET_GEN_CCMP_NEXT +@hook TARGET_GEN_MEMSET_SCRATCH_RTX + @hook TARGET_LOOP_UNROLL_ADJUST @defmac POWI_MAX_MULTS diff --git a/gcc/optabs.def b/gcc/optabs.def index b192a9d070b..643d2d17a3b 100644 --- a/gcc/optabs.def +++ b/gcc/optabs.def @@ -453,3 +453,5 @@ OPTAB_DC (vec_series_optab, "vec_series$a", VEC_SERIES) OPTAB_D (vec_shl_insert_optab, "vec_shl_insert_$a") OPTAB_D (len_load_optab, "len_load_$a") OPTAB_D (len_store_optab, "len_store_$a") + +OPTAB_DC (vec_const_duplicate_optab, "vec_const_duplicate$a", VEC_DUPLICATE) diff --git a/gcc/target.def b/gcc/target.def index 1dffedc81e4..b89e7c24471 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -2724,6 +2724,13 @@ DEFHOOK rtx, (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev, int cmp_code, tree op0, tree op1, int bit_code), NULL) +DEFHOOK +(gen_memset_scratch_rtx, + "This hook should return an rtx for scratch register in @var{mode} to\n\ +be used by memset broadcast. The default is @code{gen_reg_rtx}.", + rtx, (machine_mode mode), + gen_reg_rtx) + /* Return a new value for loop unroll size. */ DEFHOOK (loop_unroll_adjust, -- 2.31.1