RE: [PATCH PR95523] aarch64:ICE in register_tuple_type，at config/aarch64/aarch64-sve-builtins.cc:3434

[Zhanghaijian (A)]

Thanks for viewing and pushing this.

Thanks,
Haijian Zhang

> -----Original Message-----
> From: Richard Sandiford [mailto:richard.sandiford@arm.com]
> Sent: Thursday, June 11, 2020 12:01 AM
> To: Zhanghaijian (A) <z.zhanghaijian@huawei.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR95523] aarch64:ICE in register_tuple_type，at
> config/aarch64/aarch64-sve-builtins.cc:3434
> 
> Hi,
> 
> "Zhanghaijian (A)" <z.zhanghaijian@huawei.com> writes:
> > This is a simple fix for pr95523.
> > When registering the tuple type in register_tuple_type, the TYPE_ALIGN
> (tuple_type) will be changed by -fpack-struct=n.
> > We need to maintain natural alignment in handle_arm_sve_h.
> > Bootstrap and tested on aarch64 Linux platform. No new regression
> witnessed.
> 
> Sorry for dropping the ball on the bugzilla PR and not replying to your comment
> there.  For the record...
> 
> IMO it's a misfeature that -fpack-struct=N and “#pragma pack (1)”
> override explicit alignments.  E.g. it means that for:
> 
>   struct __attribute__((packed)) s1 { __attribute__((aligned(2))) int x; };
>   #pragma pack (1)
>   struct s2 { __attribute__((aligned(2))) int x; };
> 
> s1 has alignment 2 but s2 has alignment 1.  There's a comment about this in
> stor-layout.c:
> 
>       /* Should this be controlled by DECL_USER_ALIGN, too?  */
>       if (mfa != 0)
> 	SET_DECL_ALIGN (decl, MIN (DECL_ALIGN (decl), mfa));
> 
> I think the condition probably should have checked DECL_USER_ALIGN.
> 
> However, there's no telling whether anything now relies on the current
> behaviour, and since this bug is about internally-defined structures, it's
> probably not a good motivating example for changing the code above.
> 
> Also, I agree the patch is a clean way of avoiding the problem, and is probably
> more robust than the DECL_USER_ALIGN thing would have been.
> 
> Pushed to master, thanks.
> 
> Richard