RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

This patch makes the return slot argument explicit whenever a the return
value of a function that returns in memory is used.  This addresses the
problem in mudflap/19319 that a variable whose address is passed as a
return slot doesn't get TREE_ADDRESSABLE, and also avoids the redundant
copy discussed in the thread about Mark's patch for 16405.

An early version of this patch performed this transformation
unconditionally, as the rtl version in expand_call does.  However, a
problem occurred to me:  if we elide both the copy from the user variable
(the NRV optimization) and the copy from the return slot (this
optimization), we can end up combining a local variable in the called
function with a globally visible object:

// Test that the NRV optimization doesn't cause a1 to change too soon.
// { dg-do run }

#ifdef __cplusplus
extern "C" void abort();
#endif

struct A
{
  int i[200];
};

struct A a1;
int i;

struct A f ()
{
  struct A a2;
  a2.i[0] = 42;
  // a1.i[0] should still be 0 until we return.
  i = a1.i[0];
  return a2;
}

int main()
{
  a1 = f();
  if (i == 42)
    abort ();
  return 0;
}

This test breaks without my patch under C++, and would break under C except
that the language-independent NRV pass seems to have been broken by rth's
changes to RETURN_EXPR gimplification.

The fix would seem to be to insert a temporary if the object on the lhs
might escape.  The patch below just checks is_gimple_non_addressable, which
I believe to be the best we can do before actual escape analysis.

Unfortunately, this will unnecessarily pessimize some code by adding an
additional copy of the return value when calling functions that don't use
the NRV optimization.  I don't see what to do about this.

Any thoughts?

2005-01-12  Jason Merrill  <jason@redhat.com>

	* gimplify.c (gimplify_modify_expr_rhs) [CALL_EXPR]: Make return
	slot explicit.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1814 bytes --]

*** gimplify.c.~1~	2005-01-11 19:13:24.000000000 -0500
--- gimplify.c	2005-01-12 21:20:27.829403345 -0500
*************** gimplify_modify_expr_rhs (tree *expr_p, 
*** 2883,2888 ****
--- 2884,2933 ----
  	  ret = GS_UNHANDLED;
  	break;
  
+       case CALL_EXPR:
+ 	/* For calls that return in memory, give *to_p as the CALL_EXPR's
+ 	   return slot so that we don't generate a temporary.  */
+ 	if (aggregate_value_p (*from_p, *from_p))
+ 	  {
+ 	    tree init = *from_p;
+ 	    tree fn = TREE_OPERAND (init, 0);
+ 	    tree args = TREE_OPERAND (init, 1);
+ 	    tree rettype = TREE_TYPE (TREE_TYPE (TREE_TYPE (fn)));
+ 	    tree arg = *to_p;
+ 	    tree type;
+ 
+ 	    /* Only use the original target if *to_p isn't already
+ 	       addressable; if its address escapes, and the called function
+ 	       uses the NRV optimization, a conforming program could see
+ 	       *to_p change before the called function returns.  */
+ 	    bool use_temp = !is_gimple_non_addressable (*to_p);
+ 	    if (use_temp)
+ 	      {
+ 		arg = create_tmp_var (rettype, "ret");
+ 		*from_p = arg;
+ 	      }
+ 
+ 	    type = TREE_TYPE (arg);
+ 	    lang_hooks.mark_addressable (arg);
+ 	    arg = build1 (ADDR_EXPR, build_pointer_type (type), arg);
+ 	    /* The return type might have different cv-quals from arg.  */
+ 	    arg = convert (build_pointer_type (rettype), arg);
+ 	    args = tree_cons (NULL_TREE, arg, args);
+ 	    init = build3 (CALL_EXPR, rettype, fn, args, NULL_TREE);
+ 	    CALL_EXPR_HAS_RETURN_SLOT_ADDR (init) = 1;
+ 
+ 	    if (use_temp)
+ 	      gimplify_and_add (init, pre_p);
+ 	    else if (want_value)
+ 	      {
+ 		gimplify_and_add (init, pre_p);
+ 		*expr_p = *to_p;
+ 	      }
+ 	    else
+ 	      *expr_p = init;
+ 	    return GS_OK;
+ 	  }
+ 
        default:
  	ret = GS_UNHANDLED;
  	break;

Re: RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Diego Novillo]

Jason Merrill wrote:

> Any thoughts?
> 
Not an objection to the patch, but this will also cause TBAA to 
introduce more alias relations with the variable in the return slot. 
The aliasing code will not only see it as an addressable variable, but 
will also consider it call-clobbered.

I'm not sure how much of a problem this is, but it should be easy to 
measure by checking the .alias dumps before/after the patch.  We 
obviously need to make the aliaser smarter (law and dberlin are working 
on enhancements that should help, though they may not be ready for 4.0).


Diego.

Re: RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Daniel Berlin]

On Wed, 12 Jan 2005, Diego Novillo wrote:

> Jason Merrill wrote:
>
>> Any thoughts?
>> 
> Not an objection to the patch, but this will also cause TBAA to introduce 
> more alias relations with the variable in the return slot. The aliasing code 
> will not only see it as an addressable variable, but will also consider it 
> call-clobbered.
>
> I'm not sure how much of a problem this is, but it should be easy to measure 
> by checking the .alias dumps before/after the patch.  We obviously need to 
> make the aliaser smarter (law and dberlin are working on enhancements that 
> should help, though they may not be ready for 4.0).

The stuff i'm working on is definitely not 4.0 material, though i'm about 
to fix the remaining regressions.

I have no idea about Jeff's work (i assume it's a much smaller amount of 
work, since he hasn't created a branch for it or anything of the sort).


>
>
> Diego.
>

Re: RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Diego Novillo]

Forgot to add.  One obvious short-term fix fix would be to notice that 
the address is being taken by the return slot and tell the aliaser to 
disregard it.


Diego.

Re: RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 189 bytes --]

This patch fixes a couple of bugs in the earlier version.

2005-01-12  Jason Merrill  <jason@redhat.com>

	* gimplify.c (gimplify_modify_expr_rhs) [CALL_EXPR]: Make return
	slot explicit.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1885 bytes --]

*** gimplify.c.~1~	2005-01-11 19:13:24.000000000 -0500
--- gimplify.c	2005-01-13 14:03:45.000000000 -0500
*************** gimplify_modify_expr_rhs (tree *expr_p, 
*** 2883,2888 ****
--- 2884,2937 ----
  	  ret = GS_UNHANDLED;
  	break;
  
+       case CALL_EXPR:
+ 	/* For calls that return in memory, give *to_p as the CALL_EXPR's
+ 	   return slot so that we don't generate a temporary.  */
+ 	if (aggregate_value_p (*from_p, *from_p))
+ 	  {
+ 	    tree init = *from_p;
+ 	    tree fn = TREE_OPERAND (init, 0);
+ 	    tree args = TREE_OPERAND (init, 1);
+ 	    tree rettype = TREE_TYPE (TREE_TYPE (TREE_TYPE (fn)));
+ 	    tree arg = *to_p;
+ 	    tree type;
+ 
+ 	    /* Only use the original target if *to_p isn't already
+ 	       addressable; if its address escapes, and the called function
+ 	       uses the NRV optimization, a conforming program could see
+ 	       *to_p change before the called function returns.  */
+ 	    bool use_temp = !is_gimple_non_addressable (*to_p);
+ 	    if (use_temp)
+ 	      {
+ 		arg = create_tmp_var (rettype, "ret");
+ 		*from_p = arg;
+ 	      }
+ 
+ 	    type = TREE_TYPE (arg);
+ 	    lang_hooks.mark_addressable (arg);
+ 	    arg = build1 (ADDR_EXPR, build_pointer_type (type), arg);
+ 	    /* The return type might have different cv-quals from arg.  */
+ 	    arg = convert (build_pointer_type (rettype), arg);
+ 	    args = tree_cons (NULL_TREE, arg, args);
+ 	    init = build3 (CALL_EXPR, rettype, fn, args, NULL_TREE);
+ 	    CALL_EXPR_HAS_RETURN_SLOT_ADDR (init) = 1;
+ 	    TREE_USED (init) = 1;
+ 
+ 	    if (use_temp)
+ 	      gimplify_and_add (init, pre_p);
+ 	    else if (want_value)
+ 	      {
+ 		gimplify_and_add (init, pre_p);
+ 		*expr_p = *to_p;
+ 	      }
+ 	    else
+ 	      *expr_p = init;
+ 	    ret = GS_OK;
+ 	  }
+ 	else
+ 	  ret = GS_UNHANDLED;
+ 	break;
+ 
        default:
  	ret = GS_UNHANDLED;
  	break;

Re: RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Mark Mitchell]

Jason Merrill wrote:

> Unfortunately, this will unnecessarily pessimize some code by adding an
> additional copy of the return value when calling functions that don't use
> the NRV optimization.  I don't see what to do about this.

Me neither.  Hopefully, most functions will use NRV...

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Richard Henderson]

On Wed, Jan 12, 2005 at 09:34:12PM -0500, Jason Merrill wrote:
> An early version of this patch performed this transformation
> unconditionally, as the rtl version in expand_call does.  However, a
> problem occurred to me:  if we elide both the copy from the user variable
> (the NRV optimization) and the copy from the return slot (this
> optimization), we can end up combining a local variable in the called
> function with a globally visible object:

Another alternative is to do this lowering later.  As late as possible,
actually.  This has a couple of advantages:

(1) Don't have to worry about optimizers thinking the object address
    leaks as with an address passed normally.

(2) Don't take the address of a variable that we might otherwise be
    able to scalarize.

(3) We'll have a much clearer picture of when we need to use a temporary,
    since we'll have true escape analysis results.


r~

Re: RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Diego Novillo]

Richard Henderson wrote:
> On Wed, Jan 12, 2005 at 09:34:12PM -0500, Jason Merrill wrote:
> 
>>An early version of this patch performed this transformation
>>unconditionally, as the rtl version in expand_call does.  However, a
>>problem occurred to me:  if we elide both the copy from the user variable
>>(the NRV optimization) and the copy from the return slot (this
>>optimization), we can end up combining a local variable in the called
>>function with a globally visible object:
> 
> 
> Another alternative is to do this lowering later.  As late as possible,
> actually.  This has a couple of advantages:
> 
Yes, but that still leaves the mudflap problem open.  pass_mudflap_1 
needs to figure out, somehow, that this particular DECL is interesting 
to it.  Perhaps mudflap ought to not rely on TREE_ADDRESSABLE alone, and 
ask whether the symbol is addressable or the target of a call that will 
force its address in the return slot.

> (1) Don't have to worry about optimizers thinking the object address
>     leaks as with an address passed normally.
> 
> (2) Don't take the address of a variable that we might otherwise be
>     able to scalarize.
> 
> (3) We'll have a much clearer picture of when we need to use a temporary,
>     since we'll have true escape analysis results.
> 
Agreed.  Exposing this too early is not kind to the optimizers.


Diego.

Re: RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Richard Henderson]

On Wed, Jan 19, 2005 at 05:04:28PM -0500, Diego Novillo wrote:
> Yes, but that still leaves the mudflap problem open.  pass_mudflap_1 
> needs to figure out...

Ah, I thought it was pass 2 that needed this.  For that perhaps we
just force the use of a temporary, let mudflap have at it, and be
done.  After all, once the address gets passed to mudflap, all of
the rest of the optimizations are moot, because the variable really
does escape.


r~

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

The patch I checked in over the weekend broke bootstrap on multiple
targets, though mysteriously not on x86_64.  This version bootstraps
cleanly on i686-pc-linux-gnu, and has no regressions on x86_64.

Applied to trunk.

2005-02-13  Jason Merrill  <jason@redhat.com>

	PR mudflap/19319, c++/19317
	* gimplify.c (gimplify_modify_expr_rhs) [CALL_EXPR]: Make return
	slot explicit.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2249 bytes --]

*** gimplify.c.~1~	2005-02-16 17:52:27.000000000 -0500
--- gimplify.c	2005-02-16 17:03:23.000000000 -0500
*************** gimplify_modify_expr_rhs (tree *expr_p, 
*** 2913,2918 ****
--- 2913,2981 ----
  	  ret = GS_UNHANDLED;
  	break;
  
+       case CALL_EXPR:
+ 	/* For calls that return in memory, give *to_p as the CALL_EXPR's
+ 	   return slot so that we don't generate a temporary.  */
+ 	if (aggregate_value_p (*from_p, *from_p))
+ 	  {
+ 	    tree init = *from_p;
+ 	    tree fn = TREE_OPERAND (init, 0);
+ 	    tree args = TREE_OPERAND (init, 1);
+ 	    tree rettype = TREE_TYPE (TREE_TYPE (TREE_TYPE (fn)));
+ 	    tree arg = *to_p;
+ 	    tree type;
+ 
+ 	    /* Only use the original target if *to_p isn't already
+ 	       addressable; if its address escapes, and the called function
+ 	       uses the NRV optimization, a conforming program could see
+ 	       *to_p change before the called function returns.  This is
+ 	       c++/19317.  */
+ 	    bool use_temp = !is_gimple_non_addressable (*to_p);
+ 
+ 	    /* A CALL_EXPR with an explicit return slot argument should
+ 	       never appear on the RHS of a MODIFY_EXPR.  */
+ 	    if (CALL_EXPR_HAS_RETURN_SLOT_ADDR (*from_p))
+ 	      abort ();
+ 
+ 	    if (use_temp)
+ 	      {
+ 		arg = create_tmp_var (rettype, "ret");
+ 		*from_p = arg;
+ 	      }
+ 
+ 	    type = TREE_TYPE (arg);
+ 	    /* FIXME: Mark the address as not escaping.  */
+ 	    lang_hooks.mark_addressable (arg);
+ 	    arg = build1 (ADDR_EXPR, build_pointer_type (type), arg);
+ 	    /* The return type might have different cv-quals from arg.  */
+ 	    arg = convert (build_pointer_type (rettype), arg);
+ 	    args = tree_cons (NULL_TREE, arg, args);
+ 	    init = build3 (CALL_EXPR, rettype, fn, args, NULL_TREE);
+ 	    CALL_EXPR_HAS_RETURN_SLOT_ADDR (init) = 1;
+ 	    TREE_USED (init) = 1;
+ 
+ 	    if (use_temp)
+ 	      {
+ 		gimplify_and_add (init, pre_p);
+ 		ret = GS_OK;
+ 		break;
+ 	      }
+ 	    else if (want_value)
+ 	      {
+ 		gimplify_and_add (init, pre_p);
+ 		*expr_p = *to_p;
+ 		return GS_OK;
+ 	      }
+ 	    else
+ 	      {
+ 		*expr_p = init;
+ 		return GS_OK;
+ 	      }
+ 	  }
+ 	else
+ 	  ret = GS_UNHANDLED;
+ 	break;
+ 
        default:
  	ret = GS_UNHANDLED;
  	break;

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Richard Henderson]

On Thu, Feb 17, 2005 at 12:34:48AM -0500, Jason Merrill wrote:
> 	PR mudflap/19319, c++/19317
> 	* gimplify.c (gimplify_modify_expr_rhs) [CALL_EXPR]: Make return
> 	slot explicit.

Breaks Alpha.

--------------------------------------
typedef int TItype __attribute__ ((mode (TI)));
typedef float DFtype __attribute__ ((mode (DF)));
extern TItype __fixunsdfti (DFtype);

TItype
__fixdfti (DFtype a)
{
  if (a < 0)
    return - __fixunsdfti (-a);
  return __fixunsdfti (a);
}
--------------------------------------

Problem is that TImode is in fact returned by reference, and is 
therefore aggregate_type_p.  But it does satisfy is_gimple_reg_type,
so we want formal temporaries lots of places.  But the formal
temporaries we create via internal_get_tmp_var wind up with 
TREE_ADDRESSABLE set, and so don't satisfy is_gimple_reg.

I tried adding an extra copy at the end of internal_get_tmp_var to
work around this, but wound up with *two* calls to __fixunsdfti
in the negation path.  Bad juju.

I'll also note for the record that we gimplify the call no less
than three times.  We ought to do something about that...


r~

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Hans-Peter Nilsson]

On Thu, 17 Feb 2005, Jason Merrill wrote:

> The patch I checked in over the weekend broke bootstrap on multiple
> targets, though mysteriously not on x86_64.  This version bootstraps
> cleanly on i686-pc-linux-gnu, and has no regressions on x86_64.

If it was such a mystery, I suggest testing on a few more
targets would have been in order, at least those reported broken
by the previous version.

> Applied to trunk.
>
> 2005-02-13  Jason Merrill  <jason@redhat.com>
>
> 	PR mudflap/19319, c++/19317
> 	* gimplify.c (gimplify_modify_expr_rhs) [CALL_EXPR]: Make return
> 	slot explicit.

Broke cris-axis-elf, failure excerpt shown below.  Will open PR
and assign it to you.

Running
/home/hp/cvs_areas/combined/combined/gcc/testsuite/gcc.c-torture/execute/execute.exp
...
FAIL: gcc.c-torture/execute/20000402-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20000511-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20010222-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20010222-1.c execution,  -O1
FAIL: gcc.c-torture/execute/20010222-1.c execution,  -O2
FAIL: gcc.c-torture/execute/20010222-1.c execution,  -O3
-fomit-frame-pointer
FAIL: gcc.c-torture/execute/20010222-1.c execution,  -O3 -g
FAIL: gcc.c-torture/execute/20010222-1.c execution,  -Os
FAIL: gcc.c-torture/execute/20020201-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20020201-1.c execution,  -O1
FAIL: gcc.c-torture/execute/20020201-1.c execution,  -O2
FAIL: gcc.c-torture/execute/20020201-1.c execution,  -O3
-fomit-frame-pointer
FAIL: gcc.c-torture/execute/20020201-1.c execution,  -O3 -g
FAIL: gcc.c-torture/execute/20020201-1.c execution,  -Os
(xpasses pruned)
FAIL: gcc.c-torture/execute/20020904-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20020904-1.c execution,  -O1
FAIL: gcc.c-torture/execute/20020904-1.c execution,  -O2
FAIL: gcc.c-torture/execute/20020904-1.c execution,  -Os
FAIL: gcc.c-torture/execute/20021119-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20021119-1.c execution,  -O1
FAIL: gcc.c-torture/execute/20021119-1.c execution,  -O2
FAIL: gcc.c-torture/execute/20021119-1.c execution,  -Os
FAIL: gcc.c-torture/execute/20021120-2.c execution,  -O0
FAIL: gcc.c-torture/execute/20021120-2.c execution,  -O1
FAIL: gcc.c-torture/execute/20021120-2.c execution,  -O2
FAIL: gcc.c-torture/execute/20021120-2.c execution,  -Os
FAIL: gcc.c-torture/execute/20021120-3.c execution,  -O0
FAIL: gcc.c-torture/execute/20021120-3.c execution,  -O1
FAIL: gcc.c-torture/execute/20021120-3.c execution,  -O2
FAIL: gcc.c-torture/execute/20021120-3.c execution,  -Os
FAIL: gcc.c-torture/execute/20030117-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20030117-1.c execution,  -O1
FAIL: gcc.c-torture/execute/20030117-1.c execution,  -O2
FAIL: gcc.c-torture/execute/20030117-1.c execution,  -Os
FAIL: gcc.c-torture/execute/20030128-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20030128-1.c execution,  -O1
FAIL: gcc.c-torture/execute/20030128-1.c execution,  -O2
FAIL: gcc.c-torture/execute/20030128-1.c execution,  -O3
-fomit-frame-pointer
FAIL: gcc.c-torture/execute/20030128-1.c execution,  -O3 -g
FAIL: gcc.c-torture/execute/20030128-1.c execution,  -Os
FAIL: gcc.c-torture/execute/20040629-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20040629-1.c execution,  -O1
FAIL: gcc.c-torture/execute/20040629-1.c execution,  -O2
FAIL: gcc.c-torture/execute/20040629-1.c execution,  -O3
-fomit-frame-pointer
FAIL: gcc.c-torture/execute/20040629-1.c execution,  -O3 -g
FAIL: gcc.c-torture/execute/20040629-1.c execution,  -Os
FAIL: gcc.c-torture/execute/20040705-1.c execution,  -O0
FAIL: gcc.c-torture/execute/20040705-1.c execution,  -O1
FAIL: gcc.c-torture/execute/20040705-1.c execution,  -O2
FAIL: gcc.c-torture/execute/20040705-1.c execution,  -O3
-fomit-frame-pointer
FAIL: gcc.c-torture/execute/20040705-1.c execution,  -O3 -g
FAIL: gcc.c-torture/execute/20040705-1.c execution,  -Os
FAIL: gcc.c-torture/execute/20040705-2.c execution,  -O0
FAIL: gcc.c-torture/execute/20040705-2.c execution,  -O1
FAIL: gcc.c-torture/execute/20040705-2.c execution,  -O2
FAIL: gcc.c-torture/execute/20040705-2.c execution,  -O3
-fomit-frame-pointer
FAIL: gcc.c-torture/execute/20040705-2.c execution,  -O3 -g
FAIL: gcc.c-torture/execute/20040705-2.c execution,  -Os
WARNING: program timed out.
FAIL: gcc.c-torture/execute/920501-2.c execution,  -O0
WARNING: program timed out.
FAIL: gcc.c-torture/execute/920501-2.c execution,  -O1
WARNING: program timed out.
FAIL: gcc.c-torture/execute/920501-2.c execution,  -O2
WARNING: program timed out.
FAIL: gcc.c-torture/execute/920501-2.c execution,  -O3
-fomit-frame-pointer
WARNING: program timed out.
FAIL: gcc.c-torture/execute/920501-2.c execution,  -O3
-fomit-frame-pointer -funroll-loops

...

brgds, H-P

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Jakub Jelinek]

On Wed, Feb 16, 2005 at 10:10:50PM -0800, Richard Henderson wrote:
> On Thu, Feb 17, 2005 at 12:34:48AM -0500, Jason Merrill wrote:
> > 	PR mudflap/19319, c++/19317
> > 	* gimplify.c (gimplify_modify_expr_rhs) [CALL_EXPR]: Make return
> > 	slot explicit.
> 
> Breaks Alpha.

Breaks s390x bootstrap as well.
../../gcc/libgcc2.c: In function '__fixdfti':
../../gcc/libgcc2.c:1254: internal compiler error: in gimplify_expr, at gimplify.c:4294
../../gcc/libgcc2.c: In function '__fixsfti':
../../gcc/libgcc2.c:1328: internal compiler error: in gimplify_expr, at gimplify.c:4294

	Jakub

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Ulrich Weigand]

Richard Henderson wrote:
> On Thu, Feb 17, 2005 at 12:34:48AM -0500, Jason Merrill wrote:
> > 	PR mudflap/19319, c++/19317
> > 	* gimplify.c (gimplify_modify_expr_rhs) [CALL_EXPR]: Make return
> > 	slot explicit.
> 
> Breaks Alpha.

And S/390.  On s390x-ibm-linux I get the same bootstrap failure
Richard describes; on s390-ibm-linux bootstrap succeeds but I
get dozens of testcase failures ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Jason Merrill]

I've reverted this patch again; I think I now have enough bug reports to
work on without leaving the tree broken.  :/

Jason

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Joseph S. Myers]

On Fri, 18 Feb 2005, Jason Merrill wrote:

> I've reverted this patch again; I think I now have enough bug reports to
> work on without leaving the tree broken.  :/

Does bug 19319 now need reopening?

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
    jsm@polyomino.org.uk (personal mail)
    joseph@codesourcery.com (CodeSourcery mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Laurent GUERBY]

Thanks, I confirm we're now back with a normal number
of Ada regressions.

http://gcc.gnu.org/ml/gcc-testresults/2005-02/msg00777.html
http://gcc.gnu.org/ml/gcc-testresults/2005-02/msg00776.html

Laurent

On Fri, 2005-02-18 at 14:54 -0500, Jason Merrill wrote:
> I've reverted this patch again; I think I now have enough bug reports to
> work on without leaving the tree broken.  :/
> 
> Jason
> 

Re: PATCH to gimplify_modify_expr_rhs to make return slot explicit

[Jason Merrill]

I'm no longer sure this patch is such a good idea.  When I looked into the
failures, they turned out to be problems with various bits of code not
dealing with CALL_EXPR_HAS_RETURN_SLOT_ADDR.  One was with code that
assumes that a call to a const function has no side-effects, which is not
true if the call has CALL_EXPR_HAS_RETURN_SLOT_ADDR set.

...which led me to think that maybe this transformation isn't a very good
idea; it makes the modification of the target less explicit, while most of
gimplification is trying to make all effects more explicit.

We do still need to make the caller-side return slot optimization explicit;
the mudflap problem illustrates this, as does c++/19317, where both caller
and callee share the return slot with another variable, leading to chaos.
Any ideas on a better way to express this?  Would making it a property of
the MODIFY_EXPR make sense?

Jason