On Fri, 28 Oct 2022, Andre Vieira (lists) wrote: > > On 24/10/2022 14:29, Richard Biener wrote: > > On Mon, 24 Oct 2022, Andre Vieira (lists) wrote: > > > >> 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 ? > > But it's not only an issue with scatter-gather, other load/store handling > > assumes it can create a pointer to the start of the access and thus > > requires BITS_PER_UNIT alignment for each of them. So we need to fail > > at data-ref analysis somehow. > > > > Richard. > > Sorry for the delay on this, had some other things come in between. After our > IRC discussion I believe we agreed that it would be neater to check this in > vect_check_gather_scatter as I did in the original patch in > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604139.html > The main reasons being that to check earlier we'd need to walk the DR_REF to > look for any FIELD_DECL that has DECL_BIT_FIELD set and we decided against > that. > > Can you confirm the original patch is OK for trunk?
Yes. Thanks, Richard. > Kind regards, > Andre