public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit
@ 2005-01-13  2:35 Jason Merrill
  2005-01-13  2:53 ` Diego Novillo
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Jason Merrill @ 2005-01-13  2:35 UTC (permalink / raw)
  To: gcc-patches; +Cc: Mark Mitchell, Frank Ch. Eigler, Richard Henderson

[-- 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;

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2005-03-04 21:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-01-13  2:35 RFC: PATCH to gimplify_modify_expr_rhs to make return slot explicit Jason Merrill
2005-01-13  2:53 ` Diego Novillo
2005-01-13  2:59   ` Daniel Berlin
2005-01-13  3:01   ` Diego Novillo
2005-01-14  1:57 ` Jason Merrill
2005-01-15  2:02 ` Mark Mitchell
2005-01-19 21:59 ` Richard Henderson
2005-01-19 22:05   ` Diego Novillo
2005-01-19 22:23     ` Richard Henderson
2005-02-17 17:44 ` Jason Merrill
2005-02-17 18:05   ` Richard Henderson
2005-02-18 14:58     ` Jakub Jelinek
2005-02-18 23:35     ` Ulrich Weigand
2005-02-17 18:07   ` Hans-Peter Nilsson
2005-02-19  0:51   ` Jason Merrill
2005-02-19  2:38     ` Joseph S. Myers
2005-02-19  4:17     ` Laurent GUERBY
2005-03-04 21:21   ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).