This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PING PATCH, PR 57748] Set mode of structures with zero sized arrays to be BLK
- From: Martin Jambor <mjambor at suse dot cz>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Jakub Jelinek <jakub at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, "Joseph S. Myers" <joseph at codesourcery dot com>
- Date: Wed, 28 Aug 2013 11:21:26 +0200
- Subject: Re: [PING PATCH, PR 57748] Set mode of structures with zero sized arrays to be BLK
- Authentication-results: sourceware.org; auth=none
- References: <20130802114531 dot GE2728 at virgil dot suse> <20130823151122 dot GC32344 at virgil dot suse> <20130823152923 dot GN1814 at tucnak dot redhat dot com> <20130827140342 dot GA3564 at virgil dot suse> <20130827140906 dot GI21876 at tucnak dot zalov dot cz> <alpine dot LNX dot 2 dot 00 dot 1308280959200 dot 20077 at zhemvz dot fhfr dot qr> <alpine dot LNX dot 2 dot 00 dot 1308281011340 dot 20077 at zhemvz dot fhfr dot qr>
On Wed, Aug 28, 2013 at 10:17:52AM +0200, Richard Biener wrote:
> On Wed, 28 Aug 2013, Richard Biener wrote:
> > Eh ... :/
> >
> > I'm extremely nervous about this change. I also believe the change
> > is unrelated to the issue in the bugreport (even if it happens to
> > fix the ICE).
> >
> > Let me have a (short) look.
>
> Everything looks ok up until
>
> if (offset != 0)
> {
> enum machine_mode address_mode;
>
> where we are supposed to factor in offset into the memory address.
> This code doesn't handle the movmisalign path which has the memory
> in 'mem' instead of in to_rtx.
>
> And indeed a hack like
>
> Index: gcc/expr.c
> ===================================================================
> --- gcc/expr.c (revision 202043)
> +++ gcc/expr.c (working copy)
> @@ -4753,6 +4753,9 @@ expand_assignment (tree to, tree from, b
> {
> enum machine_mode address_mode;
> rtx offset_rtx;
> + rtx saved_to_rtx = to_rtx;
> + if (misalignp)
> + to_rtx = mem;
>
> if (!MEM_P (to_rtx))
> {
> @@ -4785,6 +4788,11 @@ expand_assignment (tree to, tree from, b
> to_rtx = offset_address (to_rtx, offset_rtx,
> highest_pow2_factor_for_target (to,
>
> offset));
> + if (misalignp)
> + {
> + mem = to_rtx;
> + to_rtx = saved_to_rtx;
> + }
> }
>
> /* No action is needed if the target is not a memory and the field
>
> fixes the testcase and results in (at -O)
>
> foo:
> .LFB1:
> .cfi_startproc
> pushq %rbx
> .cfi_def_cfa_offset 16
> .cfi_offset 3, -16
> movq %rdi, %rbx
> call get_i
> cltq
> addq $1, %rax
> salq $4, %rax
> movdqa .LC0(%rip), %xmm0
> movdqu %xmm0, (%rbx,%rax)
> popq %rbx
> .cfi_def_cfa_offset 8
> ret
>
> exactly what is expected.
>
> Martin, do you want to audit the (few) places we do the movmisalign
> game for the same problem or shall I put it on my TODO? The audit
> should look for all MEM_P (to_rtx) tests which really should look
> at 'mem' for the unaligned move case (I see a volatile bitfiled
> case for example ...).
And I thought I had an easy way out. Well, let me see what I can do.
Thanks,
Martin