From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by sourceware.org (Postfix) with ESMTPS id CD7ED394FC14 for ; Mon, 28 Jun 2021 08:38:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CD7ED394FC14 Received: by mail-ed1-x52d.google.com with SMTP id h17so24579880edw.11 for ; Mon, 28 Jun 2021 01:38: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:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xPK8x7s7OcAKarjP+Zo6KnRUqEzzhXL22di++D1h9G8=; b=g7RU7X3cjtYVUnrryKEm0sTCZ+yCiw3wR3TkG+cFl4AgTXI3O958N1r1QSb/0lH2SE PhKTcudnFJZwHMsby8eQzTkIYB+F0ZVvSHtABXRAX0dLzBjH2nWZCRIjkqXwLu2mURla 1Sfc7GdJqWQlmccK4HfHIsFeLcB9xETM3v+n9vfM2G+U/cwj74SMs6YRfpuPtjIcPMMX U++zlN40bwMkWoP0REsAMnVFB3PVHJ0KBNePFgTl/TiuZS4Z5ue1teHeVypD46k4JhnF Ii96RMxOvobtNIaFl9maCGZGDotab+mvULOo4bKprHihrDPA7zCcTEWxTDiM4ELcdH73 uf3w== X-Gm-Message-State: AOAM533BHZr7LSrs4K76gY0QqRTUctHIODBw41xo1io1KPu19IjG6vpn vwz17i9QwDcqG35Q6Y25kaEDQSTZPql+5evFbwBtdw== X-Google-Smtp-Source: ABdhPJyc604duFPY1nII0ct2EcQ6kk+R8uGcJmtEZ8cxbedPOIlyhlFmKpQi7QKVH5X0Pg0v5orLq/siVb35NymObDA= X-Received: by 2002:aa7:cd85:: with SMTP id x5mr1866113edv.115.1624869514764; Mon, 28 Jun 2021 01:38:34 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Prathamesh Kulkarni Date: Mon, 28 Jun 2021 14:07:58 +0530 Message-ID: Subject: Re: [ARM] PR98435: Missed optimization in expanding vector constructor To: Kyrylo Tkachov Cc: Christophe Lyon , gcc Patches Content-Type: multipart/mixed; boundary="0000000000003c070105c5cf6a36" X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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, 28 Jun 2021 08:38:38 -0000 --0000000000003c070105c5cf6a36 Content-Type: text/plain; charset="UTF-8" On Thu, 24 Jun 2021 at 22:01, Kyrylo Tkachov wrote: > > > > > -----Original Message----- > > From: Prathamesh Kulkarni > > Sent: 14 June 2021 09:02 > > To: Christophe Lyon > > Cc: gcc Patches ; Kyrylo Tkachov > > > > Subject: Re: [ARM] PR98435: Missed optimization in expanding vector > > constructor > > > > On Wed, 9 Jun 2021 at 15:58, Prathamesh Kulkarni > > wrote: > > > > > > On Fri, 4 Jun 2021 at 13:15, Christophe Lyon > > wrote: > > > > > > > > On Fri, 4 Jun 2021 at 09:27, Prathamesh Kulkarni via Gcc-patches > > > > wrote: > > > > > > > > > > Hi, > > > > > As mentioned in PR, for the following test-case: > > > > > > > > > > #include > > > > > > > > > > bfloat16x4_t f1 (bfloat16_t a) > > > > > { > > > > > return vdup_n_bf16 (a); > > > > > } > > > > > > > > > > bfloat16x4_t f2 (bfloat16_t a) > > > > > { > > > > > return (bfloat16x4_t) {a, a, a, a}; > > > > > } > > > > > > > > > > Compiling with arm-linux-gnueabi -O3 -mfpu=neon -mfloat-abi=softfp > > > > > -march=armv8.2-a+bf16+fp16 results in f2 not being vectorized: > > > > > > > > > > f1: > > > > > vdup.16 d16, r0 > > > > > vmov r0, r1, d16 @ v4bf > > > > > bx lr > > > > > > > > > > f2: > > > > > mov r3, r0 @ __bf16 > > > > > adr r1, .L4 > > > > > ldrd r0, [r1] > > > > > mov r2, r3 @ __bf16 > > > > > mov ip, r3 @ __bf16 > > > > > bfi r1, r2, #0, #16 > > > > > bfi r0, ip, #0, #16 > > > > > bfi r1, r3, #16, #16 > > > > > bfi r0, r2, #16, #16 > > > > > bx lr > > > > > > > > > > This seems to happen because vec_init pattern in neon.md has VDQ > > mode > > > > > iterator, which doesn't include V4BF. In attached patch, I changed > > > > > mode > > > > > to VDQX which seems to work for the test-case, and the compiler now > > generates: > > > > > > > > > > f2: > > > > > vdup.16 d16, r0 > > > > > vmov r0, r1, d16 @ v4bf > > > > > bx lr > > > > > > > > > > However, the pattern is also gated on TARGET_HAVE_MVE and I am > > not > > > > > sure if either VDQ or VDQX are correct modes for MVE since MVE has > > > > > only 128-bit vectors ? > > > > > > > > > > > > > I think patterns common to both Neon and MVE should be moved to > > > > vec-common.md, I don't know why such patterns were left in neon.md. > > > Since we end up calling neon_expand_vector_init for both NEON and MVE, > > > I am not sure if we should separate the pattern ? > > > Would it make sense to FAIL if the mode size isn't 16 bytes for MVE as > > > in attached patch so > > > it will call neon_expand_vector_init only for 128-bit vectors ? > > > Altho hard-coding 16 in the pattern doesn't seem a good idea to me either. > > ping https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572342.html > > (attaching patch as text). > > > > --- a/gcc/config/arm/neon.md > +++ b/gcc/config/arm/neon.md > @@ -459,10 +459,12 @@ > ) > > (define_expand "vec_init" > - [(match_operand:VDQ 0 "s_register_operand") > + [(match_operand:VDQX 0 "s_register_operand") > (match_operand 1 "" "")] > "TARGET_NEON || TARGET_HAVE_MVE" > { > + if (TARGET_HAVE_MVE && GET_MODE_SIZE (GET_MODE (operands[0])) != 16) > + FAIL; > neon_expand_vector_init (operands[0], operands[1]); > DONE; > }) > > I think we should move this to vec-common.md like Christophe said. > Perhaps rather than making it FAIL for non-16 MVE sizes we just disable it in the expander condition? > "TARGET_NEON || (TARGET_HAVE_MVE && GET_MODE_SIZE (< VDQ>mode) != 16)" Is it OK to use mode ? Because using mode resulted in lot of build errors. Also, I think the comparison should be inverted, ie, GET_MODE_SIZE (mode) == 16 since we want to make the pattern pass if target is MVE and vector size is 16 bytes ? Do these changes in attached patch look OK ? Thanks, Prathamesh > > Thanks, > Kyrill > > > Thanks, > > Prathamesh > > > > > > Thanks, > > > Prathamesh > > > > > > > > That being said, I suggest you look at other similar patterns in > > > > vec-common.md, most of which are gated on > > > > ARM_HAVE__ARITH > > > > and possibly beware of issues with iwmmxt :-) > > > > > > > > Christophe > > > > > > > > > Thanks, > > > > > Prathamesh --0000000000003c070105c5cf6a36 Content-Type: application/octet-stream; name="pr98435-3.diff" Content-Disposition: attachment; filename="pr98435-3.diff" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_kqgda7ak0 ZGlmZiAtLWdpdCBhL2djYy9jb25maWcvYXJtL25lb24ubWQgYi9nY2MvY29uZmlnL2FybS9uZW9u Lm1kCmluZGV4IDZhNjU3MzMxN2NmLi4wYzk4YjNhOGYyMyAxMDA2NDQKLS0tIGEvZ2NjL2NvbmZp Zy9hcm0vbmVvbi5tZAorKysgYi9nY2MvY29uZmlnL2FybS9uZW9uLm1kCkBAIC00NTgsMTUgKzQ1 OCw2IEBACiAgIFsoc2V0X2F0dHIgInR5cGUiICJuZW9uX3N0b3JlMV9vbmVfbGFuZV9xLG5lb25f dG9fZ3BfcSIpXQogKQogCi0oZGVmaW5lX2V4cGFuZCAidmVjX2luaXQ8bW9kZT48Vl9lbGVtX2w+ IgotICBbKG1hdGNoX29wZXJhbmQ6VkRRIDAgInNfcmVnaXN0ZXJfb3BlcmFuZCIpCi0gICAobWF0 Y2hfb3BlcmFuZCAxICIiICIiKV0KLSAgIlRBUkdFVF9ORU9OIHx8IFRBUkdFVF9IQVZFX01WRSIK LXsKLSAgbmVvbl9leHBhbmRfdmVjdG9yX2luaXQgKG9wZXJhbmRzWzBdLCBvcGVyYW5kc1sxXSk7 Ci0gIERPTkU7Ci19KQotCiA7OyBEb3VibGV3b3JkIGFuZCBxdWFkd29yZCBhcml0aG1ldGljLgog CiA7OyBOT1RFOiBzb21lIG90aGVyIGluc3RydWN0aW9ucyBhbHNvIHN1cHBvcnQgNjQtYml0IGlu dGVnZXIKZGlmZiAtLWdpdCBhL2djYy9jb25maWcvYXJtL3ZlYy1jb21tb24ubWQgYi9nY2MvY29u ZmlnL2FybS92ZWMtY29tbW9uLm1kCmluZGV4IDhlMzUxNTFkYTQ2Li43YTkxODdiZjBlNCAxMDA2 NDQKLS0tIGEvZ2NjL2NvbmZpZy9hcm0vdmVjLWNvbW1vbi5tZAorKysgYi9nY2MvY29uZmlnL2Fy bS92ZWMtY29tbW9uLm1kCkBAIC01NjUsMyArNTY1LDEyIEBACiAKICAgRE9ORTsKIH0pCisKKyhk ZWZpbmVfZXhwYW5kICJ2ZWNfaW5pdDxtb2RlPjxWX2VsZW1fbD4iCisgIFsobWF0Y2hfb3BlcmFu ZDpWRFFYIDAgInNfcmVnaXN0ZXJfb3BlcmFuZCIpCisgICAobWF0Y2hfb3BlcmFuZCAxICIiICIi KV0KKyAgIlRBUkdFVF9ORU9OIHx8IChUQVJHRVRfSEFWRV9NVkUgJiYgR0VUX01PREVfU0laRSAo PE1PREU+bW9kZSkgPT0gMTYpIgoreworICBuZW9uX2V4cGFuZF92ZWN0b3JfaW5pdCAob3BlcmFu ZHNbMF0sIG9wZXJhbmRzWzFdKTsKKyAgRE9ORTsKK30pCg== --0000000000003c070105c5cf6a36--