On Sat, 13 May 2017, Eric Botcazou wrote:

> > Does this happen on the GCC7 branch as well?  The patch just guards an
> > indirect ref folding (I refrained from trying to make it correct given I
> > think it's premature optimization).
> 
> No, mainline and GCC 7 branch are fine.  It appears that the folding 
> (probably 
> to BIT_FIELD_REF) is necessary to break apart the store on the 6 branch.
> 
> > I'll try to investigate on Monday if you don't beat me to it.  Feel free to
> > revert the backport in the meantime.
> 
> No urgency, it's rather marginal.
> 
> > Note I think you can trigger the same bug with some source changes
> > independent of the patch which means the GENERIC must be somehow invalid.
> 
> Possibly so, yes.

So on the branch the frontends generate

  *((long long int * {ref-all}) &p->b + 8) = 5;

that's bogus to the effect that the dereference happens in type
'long long int' rather than an unaligned variant of it.  On the
GCC 7 branch and trunk we now generate

  VIEW_CONVERT_EXPR<long long int[2]>(p->b)[1] = 5;

which does not introduce an artificial dereference and does the folding
(even for variable indices) directly here.  Which also hints at that
we mishandle

typedef long long V
__attribute__ ((vector_size (2 * sizeof (long long)), may_alias));

typedef struct S { V b; } P __attribute__((aligned (1)));

__attribute__((noinline, noclone)) void
bar (P *p, int i)
{
  p->b[i] = 5;
}

even before the patch:

  *((long long int * {ref-all}) &p->b + (sizetype) ((unsigned int) i * 8)) 
= 5;

which cannot be folded into a BIT_FIELD_REF but we end up with

  V * {ref-all} D.1412;
  unsigned int i.0;
  unsigned int D.1414;
  long long int * {ref-all} D.1415;

  D.1412 = &p->b;
  i.0 = (unsigned int) i;
  D.1414 = i.0 * 8;
  D.1415 = D.1412 + D.1414;
  *D.1415 = 5;

and

bar:
        save    %sp, -96, %sp
        st      %i0, [%fp+68]
        st      %i1, [%fp+72]
        ld      [%fp+68], %g2
        ld      [%fp+72], %g1
        sll     %g1, 3, %g1
        add     %g2, %g1, %g1
        mov     0, %g2
        mov     5, %g3
        std     %g2, [%g1]
        nop
        restore
        jmp     %o7+8

which (not knowing SPARC assembly) looks bogus as well.

At this point I'll favor simply reverting the patch from the branch
rather than trying to backport the IL change for GENERIC.

Thus consider that done.

Richard.

Reply via email to