From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 11E083858421; Mon, 24 Oct 2022 13:24:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 11E083858421 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E5FBBED1; Mon, 24 Oct 2022 06:24:17 -0700 (PDT) Received: from [10.1.27.136] (E121495.arm.com [10.1.27.136]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 151FC3F7B4; Mon, 24 Oct 2022 06:24:10 -0700 (PDT) Message-ID: <3cc86b91-eef3-7def-7b3b-464c58369f4b@arm.com> Date: Mon, 24 Oct 2022 14:24:06 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.3.3 Subject: Re: vect: Make vect_check_gather_scatter reject offsets that aren't multiples of BITS_PER_UNIT [PR107346] Content-Language: en-US To: Richard Biener Cc: "gcc-patches@gcc.gnu.org" , Richard Sandiford , ebotcazou@gcc.gnu.org References: <129db1b0-0d2a-b768-bc80-9f73d665e8f8@arm.com> From: "Andre Vieira (lists)" In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.1 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,NICE_REPLY_A,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 24/10/2022 13:46, Richard Biener wrote: > On Mon, 24 Oct 2022, Andre Vieira (lists) wrote: > >> On 24/10/2022 08:17, Richard Biener wrote: >>> Can you check why vect_find_stmt_data_reference doesn't trip on the >>> >>> if (TREE_CODE (DR_REF (dr)) == COMPONENT_REF >>> && DECL_BIT_FIELD (TREE_OPERAND (DR_REF (dr), 1))) >>> { >>> free_data_ref (dr); >>> return opt_result::failure_at (stmt, >>> "not vectorized:" >>> " statement is an unsupported" >>> " bitfield access %G", stmt); >>> } >> It used to, which is why this test didn't trigger the error before my patch, >> but we lower it to BIT_FIELD_REFs in ifcvt now so it is no longer a >> DECL_BIT_FIELD. >> >> But that is a red-herring, if you change the test structure's 'type Int24 is >> mod 2**24;' to 'type Int24 is mod 2**32;', thus making the field we access a >> normal 32-bit integer, the field no longer is a DECL_BIT_FIELD and thus my >> lowering does nothing. However, you will still get the failure because the >> field before it is a packed 4-bit field, making the offset to the field we are >> accessing less than BITS_PER_UNIT. > Hmm, so the _intent_ of DECL_BIT_FIELD_REPRESENTATIVE is to definitely > _not_ be a DECL_BIT_FIELD (well, that's the whole point!). So this > shows an issue with setting up DECL_BIT_FIELD_REPRESENTATIVE? Of course > for a type with an alignment less than BITS_PER_UNIT (is StructB actually > such a type?) there cannot be a representative that isn't, so maybe > we should then set DECL_BIT_FIELD on it with a condition like Eric > mentions? I could do this, but it would not resolve the latent issue as I could still reproduce it without using any of the bitfield lowering code, see below. > >>> ? I think we should amend this check and I guess that >>> checking multiple_p on DECL_FIELD_BIT_OFFSET should be enough? >> That won't work either, unless we do the same walk-through the full access as >> we do in get_inner_reference. > I suppose we should not "if-convert" bit field accesses with a > DECL_BIT_FIELD representative. There isn't any benefit doing that > (not for general bitfield lowering either). Changing if-convert would merely change this testcase but we could still trigger using a different structure type, changing the size of Int24 to 32 bits rather than 24: package Loop_Optimization23_Pkg is   type Nibble is mod 2**4;   type Int24  is mod 2**32;  -- Changed this from 24->32   type StructA is record     a : Nibble;     b : Int24;   end record;   pragma Pack(StructA);   type StructB is record     a : Nibble;     b : StructA;   end record;   pragma Pack(StructB);   type ArrayOfStructB is array(0..100) of StructB;   procedure Foo (X : in out ArrayOfStructB); end Loop_Optimization23_Pkg; This would yield a DR_REF (dr): (*x_7(D))[_1].b.b  where the last 'b' isn't a DECL_BIT_FIELD anymore, but the first one still is and still has the non-multiple of BITS_PER_UNIT offset. Thus passing the vect_find_stmt_data_reference check and triggering the vect_check_gather_scatter failure. So unless we go and make sure we always set the DECL_BIT_FIELD on all subsequent accesses of a DECL_BIT_FIELD 'struct' (which is odd enough on its own) then we are better off catching the issue in vect_check_gather_scatter ?