PATCH for store_expr

[Mark Mitchell <mark at codesourcery dot com>]

In exploring the semantics of statement-expressions, I wandered across
this oddity in store_expr.  I believe it to be a bug, and this to be
the obvious fix, so assuming test passage, I will put in this change.
Of course, if anyone thinks I'm wrong, shout.

In store_expr, we did:

  !       temp = expand_expr (exp, cse_not_expected ? NULL_RTX : target,
  ! 			      GET_MODE (target), 0);

However, whether or not we pass down a TARGET makes a world of
difference.  In particular, if EXP is a TARGET_EXPR, then if there's
no TARGET, we generate a cleanup for it.  That can result in too many
cleanups, if we were supposed to be storing the TARGET_EXPR into a
variable, which has already had a cleanup generated for it.

Thus, the setting of `cse_not_expected' is affecting not just the
quality of the code generated, but the semantics of the generated
code.  That can't be right.

It's hard to give a good test-case for this.  Here's one:

  extern "C" void printf (const char*, ...);

  int i;

  struct A 
  {
    A () { printf ("Constructing %x\n", this); ++i; }
    ~A () { printf ("Destroying %x\n", this); --i; }
  };

  A a;
  A f () { return a; }

  int main ()
  {
    ({ const A& ar = f (); });
    return 0;
  }

This test-case prints:

  Constructing 807e1f8
  Destroying bffffa12
  Destroying bffffa13
  Destroying 807e1f8

Astute readers will note that there is no copy constructor, and should
therefore be asking "are there two copies being made, that just aren't
being reported?"  The problem is that declaring a copy constructor
gives `A' BLKmode, which works around the bug.  But, there is only one
copy being made.  It is the copy from the `return' in `f', where `a'
is returned by value.  That value is bound directly to the reference
`ar', and should be destroyed at the close of the block in the
statement-expression.

One way of convincing yourself that something odd is going on is to
note that removing the declaration from the statement-expression, so
that `main' is just:

  int main ()
  {
    { const A& ar = f (); }
    return 0;
  }

gives different output:

  Constructing 807e1b8
  Destroying bffffa13
  Destroying 807e1b8

which doesn't make sense; surely wrapping a pair of parentheses around
that block shouldn't change the number of temporaries created.

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

Mon Aug 23 22:28:16 1999  Mark Mitchell  <mark@codesourcery.com>

	* expr.c (store_expr): Always pass down the target, even when not
	doing CSE.

Index: expr.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/expr.c,v
retrieving revision 1.161
diff -c -p -r1.161 expr.c
*** expr.c	1999/08/18 17:51:23	1.161
--- expr.c	1999/08/24 05:22:25
*************** store_expr (exp, target, want_value)
*** 3577,3584 ****
         Don't do this if TARGET is volatile because we are supposed
         to write it and then read it.  */
      {
!       temp = expand_expr (exp, cse_not_expected ? NULL_RTX : target,
! 			  GET_MODE (target), 0);
        if (GET_MODE (temp) != BLKmode && GET_MODE (temp) != VOIDmode)
  	temp = copy_to_reg (temp);
        dont_return_target = 1;
--- 3577,3583 ----
         Don't do this if TARGET is volatile because we are supposed
         to write it and then read it.  */
      {
!       temp = expand_expr (exp, target, GET_MODE (target), 0);
        if (GET_MODE (temp) != BLKmode && GET_MODE (temp) != VOIDmode)
  	temp = copy_to_reg (temp);
        dont_return_target = 1;