Re: PATCH: RTL_EXPR vs. inlining-on-trees

[Mark Mitchell <mark at codesourcery dot com>]

  This removal will have a very large cost for Ada.

That's a valuable consideration.

  It would seem that when you have tree-based inlining you *also* want to
  have the scope: shouldn't all the temporaries from one inlined function
  be allowed to overlap those from another inlined function?

Yes, but consider this:

  S f() { return S(); }
  S g() { return f(); }
  void h () { S s = g (); }

Since we actually return structures by passing a pointer to a spot
created in the caller, `g' creates a temporary.  In C++, we cleverly
use TARGET_EXPRs to avoid extra copies, so given this situation we
create only one temoprary; there is no additional temporary created in
`h'.  So, if the `g' temporary is destroyed by the time we get back to
`h', we're in trouble.

  Can you say more about why you think this feature is problematical?

Yes -- in general, concepts in programs should serve one function. (*)
RTL_EXPRs, as documented are:

  /* Represents something whose RTL has already been expanded
     as a sequence which should be emitted when this expression is
     expanded.  The first operand is the RTL to emit.  It is the first
     of a chain of insns.  The second is the RTL expression for the
     result.  */
  
That's clear, coherent, and simple.  There's nothing in there about
providing an implicit scope.  If you want that functionality, you
could provide a SCOPE_EXPR; the C++ front-end essentially has such a
thing already, and I expect that will percolate to the general
tree.def file relatively soon.  

  The Ada issue is when you want to generate code at one point but insert
  it into the tree in another you do this with an RTL_EXPR.  Any temporaries
  you make during that execution are part of that code and are not
  global within the frame.

I don't claim to fully understand the complexities of what you say.
But, you could accomplish the same thing by creating 

  If there's a bug here, let's fix it, but removing this critical functionality
  seems the wrong approach to me.

It's a design bug, not a coding bug.  As I indicated in my mail, it
would be possible to add a flag to RTL_EXPRs to indicate whether or
not they have an implicit scope.  If you really think that's worth the
additional complexity, it's easy: just revert my patch, add the flag,
and conditionalize the two lines I removed in expand_expr on the flag
being set.

However, it seems that you could also arrange simply to have the
front-end call push_temp_slots and pop_temp_slots before and after
creating the RTL expression.  That's a little bit of work in the Ada
front-end, but it avoids cluttering up the middle-end with
functionality that is already available.

The point is that no real functionality, critical or otherwise, is
being removed.  You can still do what you want -- but you have to make
it clear that that's what you're doing.  Meanwhile, a useful, more
general concept (namely, embedding RTL in the tree) is supported
without limitation.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

(*) I've noticed that you are a good devil's advocate for any
generalization.  I'm sure you can come up with all kinds of
counter-arguments. :-)