On Fri, 23 Mar 2018, Jakub Jelinek wrote:
> On Fri, Mar 23, 2018 at 09:53:53AM +0100, Richard Biener wrote:
> > > Something I wasn't really aware, apparently we allow extern vars with
> > > incomplete types in "m" and "=m" constrained asm. The problem is that
> > > incomplete vars have VOIDmode mode and so their MEMs do as well,
> > > apparently
> > > everything works with it except a 2013-ish assert in alias.c.
> > >
> > > The following patch just makes sure x_mode is not VOIDmode if x doesn't
> > > have
> > > VOIDmode, if x has VOIDmode, then it is fine for x_mode to be VOIDmode
> > > too.
> > >
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > OK.
> >
> > The question now is whether we handle dependences correctly for
> > VOIDmode MEMs? Do they have !MEM_SIZE_KNOWN_P?
>
> They have MEM_SIZE_KNOWN_P true unfortunately.
> set_mem_attributes_minus_bitpos starts with:
> defattrs = mode_mem_attrs[(int) GET_MODE (ref)];
> gcc_assert (!defattrs->expr);
> gcc_assert (!defattrs->offset_known_p);
>
> /* Respect mode size. */
> attrs.size_known_p = defattrs->size_known_p;
> attrs.size = defattrs->size;
> and because both TYPE_SIZE_UNIT and DECL_SIZE_UNIT are NULL, nothing updates
> the size_known_p.
> Wonder if we just shouldn't make mode_mem_attrs[VOIDmode]->size_known_p
> false and mode_mem_attrs[VOIDmode]->size NULL, like we do for BLKmode?
>
> for (i = 0; i < (int) MAX_MACHINE_MODE; i++)
> {
> mode = (machine_mode) i;
> attrs = ggc_cleared_alloc<mem_attrs> ();
> attrs->align = BITS_PER_UNIT;
> attrs->addrspace = ADDR_SPACE_GENERIC;
> - if (mode != BLKmode)
> + if (mode != BLKmode && mode != VOIDmode)
> {
> attrs->size_known_p = true;
> attrs->size = GET_MODE_SIZE (mode);
> if (STRICT_ALIGNMENT)
> attrs->align = GET_MODE_ALIGNMENT (mode);
> }
>
> ?
Yes, that sounds like a very good idea. Pre-approved if it passes
testing.
Richard.
