From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-xe2f.google.com (mail-vs1-xe2f.google.com [IPv6:2607:f8b0:4864:20::e2f]) by sourceware.org (Postfix) with ESMTPS id 9EAA73890039 for ; Wed, 21 Jul 2021 07:52:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9EAA73890039 Received: by mail-vs1-xe2f.google.com with SMTP id o19so969057vsn.3 for ; Wed, 21 Jul 2021 00:52:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ixCtXlF5jPmhqK+chn69Wqu4NeUFdA3R+ddQ/oEYI+E=; b=rgp/9i4pbKYLeKt4bm0F8ve1/PBEaQOrdy/t9+OPpJSaoqM2LSs/41zx/ZNT8OCjHf XnXu9BttVT2dtGdSyDS+Y/+t/s2fpdI6Ati6DAZlz/cIrshlWUJEqaWMjAn2Hnxgxi04 tE0QR8Xqw5nZXRLLSgl85AyJIeKv8k6i7nZLeZ788ePAHRT3H/e6I+pmEvre7ykDYoRe XnFBVeiFXgAKeco9UQIhZGgdR3Y5NteqzntFX38ADp3J2jwAz58IIQBY87qbYtr1uE7D +96ztg/o3xks/yNK4c+ffX3bXjEarrRuoc8BMzr3M5HXiFlT1zaqQ3mmsqoZO+28Xoxi KSHw== X-Gm-Message-State: AOAM5324NtN7H99w+8tkkb14a0XL5UhUTEeMd0ThprMBr4ZZpKDfT5Ur 0n+eEgDLX1x0o38MkwKBiJs9kQINzatfMb5/dWM= X-Google-Smtp-Source: ABdhPJya9VmIKwFfucJGPoFLI1RaMG57gYvw5aNkzeRrUBR6Jrup67EYyP6jz950qbvSRyOTXiLAtg+lGUg1rPJPL7c= X-Received: by 2002:a05:6102:34eb:: with SMTP id bi11mr33851614vsb.5.1626853934240; Wed, 21 Jul 2021 00:52:14 -0700 (PDT) MIME-Version: 1.0 References: <73rrp0p-859r-oq2n-pss7-6744807s3qr5@fhfr.qr> In-Reply-To: From: Hongtao Liu Date: Wed, 21 Jul 2021 15:57:40 +0800 Message-ID: Subject: Re: [PATCH 2/2][RFC] Add loop masking support for x86 To: Richard Biener Cc: Richard Sandiford , Hongtao Liu , GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_NUMSUBJECT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Wed, 21 Jul 2021 07:52:16 -0000 On Tue, Jul 20, 2021 at 3:38 PM Richard Biener wrote: > > On Tue, 20 Jul 2021, Hongtao Liu wrote: > > > On Fri, Jul 16, 2021 at 5:11 PM Richard Biener wrote: > > > > > > On Thu, 15 Jul 2021, Richard Biener wrote: > > > > > > > On Thu, 15 Jul 2021, Richard Biener wrote: > > > > > > > > > OK, guess I was more looking at > > > > > > > > > > #define N 32 > > > > > int foo (unsigned long *a, unsigned long * __restrict b, > > > > > unsigned int *c, unsigned int * __restrict d, > > > > > int n) > > > > > { > > > > > unsigned sum = 1; > > > > > for (int i = 0; i < n; ++i) > > > > > { > > > > > b[i] += a[i]; > > > > > d[i] += c[i]; > > > > > } > > > > > return sum; > > > > > } > > > > > > > > > > where we on x86 AVX512 vectorize with V8DI and V16SI and we > > > > > generate two masks for the two copies of V8DI (VF is 16) and one > > > > > mask for V16SI. With SVE I see > > > > > > > > > > punpklo p1.h, p0.b > > > > > punpkhi p2.h, p0.b > > > > > > > > > > that's sth I expected to see for AVX512 as well, using the V16SI > > > > > mask and unpacking that to two V8DI ones. But I see > > > > > > > > > > vpbroadcastd %eax, %ymm0 > > > > > vpaddd %ymm12, %ymm0, %ymm0 > > > > > vpcmpud $6, %ymm0, %ymm11, %k3 > > > > > vpbroadcastd %eax, %xmm0 > > > > > vpaddd %xmm10, %xmm0, %xmm0 > > > > > vpcmpud $1, %xmm7, %xmm0, %k1 > > > > > vpcmpud $6, %xmm0, %xmm8, %k2 > > > > > kortestb %k1, %k1 > > > > > jne .L3 > > > > > > > > > > so three %k masks generated by vpcmpud. I'll have to look what's > > > > > the magic for SVE and why that doesn't trigger for x86 here. > > > > > > > > So answer myself, vect_maybe_permute_loop_masks looks for > > > > vec_unpacku_hi/lo_optab, but with AVX512 the vector bools have > > > > QImode so that doesn't play well here. Not sure if there > > > > are proper mask instructions to use (I guess there's a shift > > > > and lopart is free). This is QI:8 to two QI:4 (bits) mask > > Yes, for 16bit and more, we have KUNPCKBW/D/Q. but for 8bit > > unpack_lo/hi, only shift. > > > > conversion. Not sure how to better ask the target here - again > > > > VnBImode might have been easier here. > > > > > > So I've managed to "emulate" the unpack_lo/hi for the case of > > > !VECTOR_MODE_P masks by using sub-vector select (we're asking > > > to turn vector(8) into two > > > vector(4) ) via BIT_FIELD_REF. That then > > > produces the desired single mask producer and > > > > > > loop_mask_38 = VIEW_CONVERT_EXPR>(loop_mask_54); > > > loop_mask_37 = BIT_FIELD_REF ; > > > > > > note for the lowpart we can just view-convert away the excess bits, > > > fully re-using the mask. We generate surprisingly "good" code: > > > > > > kmovb %k1, %edi > > > shrb $4, %dil > > > kmovb %edi, %k2 > > > > > > besides the lack of using kshiftrb. I guess we're just lacking > > > a mask register alternative for > > Yes, we can do it similar as kor/kand/kxor. > > > > > > (insn 22 20 25 4 (parallel [ > > > (set (reg:QI 94 [ loop_mask_37 ]) > > > (lshiftrt:QI (reg:QI 98 [ loop_mask_54 ]) > > > (const_int 4 [0x4]))) > > > (clobber (reg:CC 17 flags)) > > > ]) 724 {*lshrqi3_1} > > > (expr_list:REG_UNUSED (reg:CC 17 flags) > > > (nil))) > > > > > > and so we reload. For the above cited loop the AVX512 vectorization > > > with --param vect-partial-vector-usage=1 does look quite sensible > > > to me. Instead of a SSE vectorized epilogue plus a scalar > > > epilogue we get a single fully masked AVX512 "iteration" for both. > > > I suppose it's still mostly a code-size optimization (384 bytes > > > with the masked epiloge vs. 474 bytes with trunk) since it will > > > be likely slower for very low iteration counts but it's good > > > for icache usage then and good for less branch predictor usage. > > > > > > That said, I have to set up SPEC on a AVX512 machine to do > > Does patch land in trunk already, i can have a test on CLX. > > I'm still experimenting a bit right now but hope to get something > trunk ready at the end of this or beginning next week. Since it's > disabled by default we can work on improving it during stage1 then. > > I'm mostly struggling with the GIMPLE IL to be used for the > mask unpacking since we currently reject both the BIT_FIELD_REF > and the VIEW_CONVERT we generate (why do AVX512 masks not all have > SImode but sometimes QImode and sometimes HImode ...). Unfortunately > we've dropped whole-vector shifts in favor of VEC_PERM but that > doesn't work well either for integer mode vectors. So I'm still > playing with my options here and looking for something that doesn't > require too much surgery on the RTL side to recover good mask > register code ... > > Another part missing is expanders for the various cond_* patterns > > OPTAB_D (cond_add_optab, "cond_add$a") > OPTAB_D (cond_sub_optab, "cond_sub$a") > OPTAB_D (cond_smul_optab, "cond_mul$a") > OPTAB_D (cond_sdiv_optab, "cond_div$a") > OPTAB_D (cond_smod_optab, "cond_mod$a") > OPTAB_D (cond_udiv_optab, "cond_udiv$a") > OPTAB_D (cond_umod_optab, "cond_umod$a") > OPTAB_D (cond_and_optab, "cond_and$a") > OPTAB_D (cond_ior_optab, "cond_ior$a") > OPTAB_D (cond_xor_optab, "cond_xor$a") > OPTAB_D (cond_ashl_optab, "cond_ashl$a") > OPTAB_D (cond_ashr_optab, "cond_ashr$a") > OPTAB_D (cond_lshr_optab, "cond_lshr$a") > OPTAB_D (cond_smin_optab, "cond_smin$a") > OPTAB_D (cond_smax_optab, "cond_smax$a") > OPTAB_D (cond_umin_optab, "cond_umin$a") > OPTAB_D (cond_umax_optab, "cond_umax$a") > OPTAB_D (cond_fma_optab, "cond_fma$a") > OPTAB_D (cond_fms_optab, "cond_fms$a") > OPTAB_D (cond_fnma_optab, "cond_fnma$a") > OPTAB_D (cond_fnms_optab, "cond_fnms$a") > > I think the most useful are those for possibly trapping ops > (will be used by if-conversion) and those for reduction operations > (add,min,max) which would enable a masked reduction epilogue. I've added cond_add/sub/max/min/smax/smin with my local patch, but I can't figure out testcases to validate them. Any ideas? > > The good thing is that I've been able to get my hands on a > Cascadelake system so I can at least test things for correctness. > > Richard. > > > > any meaningful measurements (I suspect with just AVX2 we're not > > > going to see any benefit from masking). Hints/help how to fix > > > the missing kshiftrb appreciated. > > > > > > Oh, and if there's only V4DImode and V16HImode data then > > > we don't go the vect_maybe_permute_loop_masks path - that is, > > > we don't generate the (not used) intermediate mask but end up > > > generating two while_ult parts. > > > > > > Thanks, > > > Richard. -- BR, Hongtao