From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62d.google.com (mail-ej1-x62d.google.com [IPv6:2a00:1450:4864:20::62d]) by sourceware.org (Postfix) with ESMTPS id 86715395BC4F for ; Tue, 29 Jun 2021 10:46:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 86715395BC4F Received: by mail-ej1-x62d.google.com with SMTP id b2so4262962ejg.8 for ; Tue, 29 Jun 2021 03:46:49 -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=KPgOxgK0y9ceKzRQ4mQTUW8nLtXkCHWiXbPRpB40yuw=; b=EOdYrFGOmunzFwy3xc6ujwJAdvYxIUiQ+J7IVqX6tMG2kW/Ad5dRw0/sG3nvLi0iOo 39wTfAa7sgQXFX/Vj3814bdgFJmeFj0HeE2zHJDBWMeKy6vY7h8JY/mKjICsAkKICeVN RvLnO3qtoPi7GnDBAt4nt8/K7FU8bZdTZhpfukj5OJBplsa44LV+dKgMgsQADXqBTEZq Hn1z+cknWiXNRJ+hoCunvh7dmadjym7hpCMvbTc5EsF3IaTokXSzLGFD8DLvAal/mQx2 eQOusrUXVK+1xymCthq0DLQIWGMXP89eiN3rugvTaxYQRvUpTnTJinOZZF6GXrxhZJ2x WKYA== X-Gm-Message-State: AOAM531XVvy3uUhvNLNgY0vG5FRoUJY6kz6lUIEwi1eZ3uaDGYRkUGpP T5ua110M0Fc7lDHMqRu1qAI8iQAeHPcvTIQxdWh7Ow== X-Google-Smtp-Source: ABdhPJyj4qy9eyN2ohykRWK/IYKa8N6QZGPM5ojH4QiJG8C1pU+IxGocNE3ow5i1yBaHVrfo8QwMMd2BNd+aybT2a8w= X-Received: by 2002:a17:906:6d59:: with SMTP id a25mr29366483ejt.83.1624963608458; Tue, 29 Jun 2021 03:46:48 -0700 (PDT) MIME-Version: 1.0 References: <9fd8bc30-f7d1-0171-4147-d570413f7a62@foss.st.com> In-Reply-To: <9fd8bc30-f7d1-0171-4147-d570413f7a62@foss.st.com> From: Prathamesh Kulkarni Date: Tue, 29 Jun 2021 16:16:12 +0530 Message-ID: Subject: Re: [ARM] PR98435: Missed optimization in expanding vector constructor To: Christophe LYON Cc: Kyrylo Tkachov , gcc Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3.0 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: Tue, 29 Jun 2021 10:46:51 -0000 On Mon, 28 Jun 2021 at 14:48, Christophe LYON wrote: > > > On 28/06/2021 10:40, Kyrylo Tkachov via Gcc-patches wrote: > > > >> -----Original Message----- > >> From: Prathamesh Kulkarni > >> Sent: 28 June 2021 09:38 > >> To: Kyrylo Tkachov > >> Cc: Christophe Lyon ; gcc Patches >> patches@gcc.gnu.org> > >> Subject: Re: [ARM] PR98435: Missed optimization in expanding vector > >> constructor > >> > >> 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 ? > > Yes, you're right. > > > Can't this be ARM_HAVE__ARITH like in most expanders in vec-common.md? > > (maybe with a && !TARGET_REALLY_IWMMXT if needed) I wonder if this should be ARM_HAVE__LDST instead since we're initializing the vector ? Thanks, Prathamesh > > > Christophe > > > > Ok. > > Thanks, > > Kyrill > > > > > >> 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