Re: [PATCH][mem-ref2] Initial MEM_REF deployment

[Richard Guenther <rguenther at suse dot de>]

On Fri, 14 May 2010, Diego Novillo wrote:

> On Mon, Mar 22, 2010 at 10:59, Richard Guenther <rguenther@suse.de> wrote:
> >
> > I have created a new branch, mem-ref2, that contains the MEM_REF work
> > that is supposed to be merged during stage1 of GCC 4.6 (I'll document
> > it soon in svn.html). ÂPatches are reviewed by me and should have
> > [mem-ref2] in the subject.
> 
> Great!  This is going to be a very significant cleanup.
> 
> > To address 1) and 2) the MEM_REF tree has an explicit type that is
> > used for TBAA. ÂIt happens to be that this needs to be a pointer
> > type (because of the TYPE_REF_CAN_ALIAS_ALL flag). ÂThus the
> > MEM_REF operation implicitly contains a pointer-conversion.
> > MEM_REF <T*, p> is *(T*)p. ÂConveniently we encode this pointer-type
> > using a tree constant operand - which conveniently gets us a
> > constant offset we can use to compact simple component references
> > as well as re-materialize all accesses get_ref_base_and_extent
> > can analyze. ÂSo, MEM_REF <p, CST> then is *(typeof(CST)p + CST).
> 
> So, MEM_REF has two different kinds of first argument?  What about
> making it a 3 operand node?  MEM_REF<type, pointer, offset>.  That's
> how the wiki documents it, but you seem to be using it differently
> here.

Well, the problem is that types do not make a good tree operand
(they cause all sorts of issues in generic code that doesn't expect
a type in such place).  So I took the simple route and retrofitted
the type into the constant offset (where the type otherwise doesn't 
matter).

It might be less obvious, but we can hide this implementation
detail in accessor functions.

So as it is now there are two relevant and one irrelevant types
in a MEM_REF tree.

1) The type of the MEM_REF tree which is the type of the rvalue
2) The type of the constant offset, which is a pointer type to
the type of the lvalue as TBAA is concerned
3) The type of the pointer argument, which does not have any
semantic meaning and its only restriction is that it should be
a pointer type

> > To address 3) the pointer operand of a MEM_REF can be an address-taking
> > operation (gimple in theory allows this for INDIRECT_REFs as well,
> > just nobody properly deals with that). ÂThe trick is to not force
> > decls addressable (and thus aliased) just because they are used
> > as MEM_REF operand wrapped inside an ADDR_EXPR. ÂThis allows a
> > similar trick I used to address PR42834 to rewrite decls used in
> > MEM_REF trees into SSA once they no longer really have their
> > address taken.
> 
> I'm not sure I agree with this aspect of the plan.  What if we simply
> allowed a VAR_DECL as a MEM_REF operand?  This would prevent us having
> to consider that ADDR_EXPR "special", which is usually a source of
> headaches.

True, but then TREE_TYPE of the first operand can be a non-pointer.
An ADDR_EXPR around a DECL sounds like a minor inconvenience.

> You say that MEM_REF<ADDR_EXPR<var>,...> should not prevent 'var' from
> being written in SSA form, but is that safe?  Is 'var' really a gimple
> register if it needs to be accessed via MEM_REF?
> 
> If we cannot consider 'var' a gimple register, then keeping the
> ADDR_EXPR is fine and we shouldn't have to treat it as a special
> ADDR_EXPR, right?

It's the same as with for example complex types - if we have any
partial set in the IL (a {REAL,IMAG}PART_EXPR on the LHS), a complex
variable cannot be rewritten into SSA form.

So there are MEM_REFs that are partial sets and MEM_REFs that are
not partial sets.  Consider that all such sets are non-partial then
the DECL referenced could be rewritten into SSA form if the DECL
does not have its address taken.  Of course an ADDR_EXPR inside
a MEM_REF should not cause it to be marked address-taken or we'd
never rewrite it into SSA form.

Basically the trick is supposed to handle

 int i, j;
 int *p = &i;
 MEM_REF <p, 0> = j_3;

where after CCP we have

 MEM_REF <&i, 0> = j_3;

which we can re-write into SSA form as

 i_2 = j_3;

> I haven't gone through the patch in detail, but it has the same
> mechanical flavour that all the tuples patches had.  So I suppose it's
> going to be one of those branches...

Yes, though it's not too bad.  But indeed it's important to get
the design correct - most other changes are going to be either
mechanical or will show weaknesses of the design.

Richard.