This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PATCH to gimplify.c for 25977 and 25979


These two PRs deal with different cases where we improperly use the LHS of an assignment as the return slot for a call on the RHS.

In 25977, I assumed that the RETURN_DECL would always be safe, but this is not the case if it is an NRV.

In 25979, looking through a *& caused us to wind up with a function where the same object is both an argument (by reference) and a local variable (by way of the RSO and NRVO). I think we can avoid this without disabling the optimization entirely, but I've disabled it for now on the basis that a wrong code regression is worse than a missed optimization regression. I'll continue to look at this.

Tested x86_64-pc-linux-gnu, applied to 4.1 and trunk.
2006-02-03  Jason Merrill  <jason@redhat.com>

	PR c++/25979
	* gimplify.c (gimplify_modify_expr_rhs): Disable *& optimization for now.

	PR middle-end/25977
	* gimplify.c (gimplify_modify_expr_rhs): It's not always safe to do RVO
	on the return slot if it's an NRV.

Index: gimplify.c
===================================================================
*** gimplify.c	(revision 110559)
--- gimplify.c	(working copy)
*************** gimplify_modify_expr_rhs (tree *expr_p, 
*** 3190,3195 ****
--- 3190,3196 ----
    while (ret != GS_UNHANDLED)
      switch (TREE_CODE (*from_p))
        {
+ #if 0
        case INDIRECT_REF:
  	{
  	  /* If we have code like 
*************** gimplify_modify_expr_rhs (tree *expr_p, 
*** 3211,3216 ****
--- 3212,3218 ----
  	    ret = GS_UNHANDLED;
  	  break;
  	}
+ #endif
  
        case TARGET_EXPR:
  	{
*************** gimplify_modify_expr_rhs (tree *expr_p, 
*** 3272,3279 ****
  	    bool use_target;
  
  	    if (TREE_CODE (*to_p) == RESULT_DECL
  		&& needs_to_live_in_memory (*to_p))
! 	      /* It's always OK to use the return slot directly.  */
  	      use_target = true;
  	    else if (!is_gimple_non_addressable (*to_p))
  	      /* Don't use the original target if it's already addressable;
--- 3274,3282 ----
  	    bool use_target;
  
  	    if (TREE_CODE (*to_p) == RESULT_DECL
+ 		&& DECL_NAME (*to_p) == NULL_TREE
  		&& needs_to_live_in_memory (*to_p))
! 	      /* It's OK to use the return slot directly unless it's an NRV. */
  	      use_target = true;
  	    else if (!is_gimple_non_addressable (*to_p))
  	      /* Don't use the original target if it's already addressable;
Index: testsuite/g++.dg/opt/nrv10.C
===================================================================
*** testsuite/g++.dg/opt/nrv10.C	(revision 0)
--- testsuite/g++.dg/opt/nrv10.C	(revision 0)
***************
*** 0 ****
--- 1,48 ----
+ // PR c++/25979
+ // Bug: we were eliding too many temporaries, so that a1 was used
+ // as both 'a' and 'x' in the second operator+.
+ // { dg-do run }
+ 
+ struct A
+ {
+     A() : data1_(0), data2_(0) {}
+     A(int i, int j) : data1_(i), data2_(j) {}
+     A operator+(int);
+     friend A operator+(int, const A&);
+     ~A() {}
+ //private:
+     int data1_;
+     int data2_;
+ };
+ 
+ extern bool x;
+ 
+ extern "C" void abort ();
+ 
+ int main()
+ {
+     A a1(1,2);
+     a1 = (x ? a1 + 3 : 3 + a1);
+     if (a1.data1_ != 3 || a1.data2_ != 2)
+       abort ();
+ }
+ 
+ bool x = false;
+ 
+ A
+ A::operator+(int i)
+ {
+     A a;
+     a = *this;
+     a.data2_ = i;
+     return a;
+ }
+ 
+ A
+ operator+(int i, const A& x)
+ {
+     A a;
+     a = x;
+     a.data1_ = i;
+     return a;
+ }
Index: testsuite/g++.dg/opt/nrv11.C
===================================================================
*** testsuite/g++.dg/opt/nrv11.C	(revision 0)
--- testsuite/g++.dg/opt/nrv11.C	(revision 0)
***************
*** 0 ****
--- 1,58 ----
+ // PR middle-end/25977
+ // Bug: We were assuming that the return slot of the current function is
+ // always safe to use as the return slot for another call, because its
+ // address cannot escape.  But its address can escape if we perform the
+ // named return value optimization.
+ 
+ // { dg-do run }
+ 
+ struct A
+ {
+     A( int left, int top, int width, int height )
+       : x1(left), y1(top), x2(left+width-1), y2(top+height-1) {}
+ 
+     //A(const A& o) : x1(o.x1), y1(o.y1), x2(o.x2), y2(o.y2) {}
+     //A& operator=(const A& o ) { x1=o.x1; y1=o.y1; x2=o.x2; y2=o.y2; return *this; }
+ 
+     A  operator&(const A &r) const
+     {
+ 	A tmp(0, 0, -1, -1);
+ 	tmp.x1 = ((r.x1) < (x1) ? (x1) : (r.x1));
+ 	tmp.x2 = ((x2) < (r.x2) ? (x2) : (r.x2));
+ 	tmp.y1 = ((r.y1) < (y1) ? (y1) : (r.y1));
+ 	tmp.y2 = ((y2) < (r.y2) ? (y2) : (r.y2));
+ 	return tmp;
+     }
+ 
+     int x1;
+     int y1;
+     int x2;
+     int y2;
+ };
+ 
+ bool operator==( const A &r1, const A &r2 )
+ {
+     return r1.x1==r2.x1 && r1.x2==r2.x2 && r1.y1==r2.y1 && r1.y2==r2.y2;
+ }
+ 
+ static A test()
+ {
+     A all = A( 0, 0, 1024, 768);
+     A a = all;
+     A r = all;
+     a = a & r;
+     return a;
+ }
+ 
+ extern "C" void abort(void);
+ 
+ int main( int argc, char ** argv )
+ {
+     A all = A( 0, 0, 1024, 768);
+     A a = test();
+ 
+     if ( ! ( a == all))
+       abort();
+ 
+     return 0;
+ }
Index: testsuite/g++.dg/opt/temp1.C
===================================================================
*** testsuite/g++.dg/opt/temp1.C	(revision 110559)
--- testsuite/g++.dg/opt/temp1.C	(working copy)
***************
*** 1,6 ****
  // PR c++/16405
  // { dg-options "-O2" } 
! // { dg-do run }
  
  // There should be exactly one temporary generated for the code in "f"
  // below when optimizing -- for the result of "b + c".  We have no
--- 1,6 ----
  // PR c++/16405
  // { dg-options "-O2" } 
! // { dg-do run { xfail *-*-* } }
  
  // There should be exactly one temporary generated for the code in "f"
  // below when optimizing -- for the result of "b + c".  We have no

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]