Author: dcoakley
Date: 2011-06-21 00:40:48 -0400 (Tue, 21 Jun 2011)
New Revision: 3659

Modified:
   trunk/osprey/be/cg/reg_live.cxx
   trunk/osprey/be/com/wn_lower.cxx
   trunk/osprey/wgen/wgen_decl.cxx
   trunk/osprey/wgen/wgen_expr.cxx
   trunk/osprey/wgen/wgen_spin_symbol.cxx
Log:
Avoid unnecessary struct copies.

For the following code, the compiler generated unnecessary struct copies
for a return value that has a size too big to be passed in registers.

  typedef struct { char big[1024]; } C;
  C gc;
  extern C bar9 (void);
  extern C check9 (void) { return bar9 (); }

In this case there were two copies and one temporary variable generated.
After this change the compiler does not generate any extra copies.

The change handles the similar cases of initialization by function
call ("C c = bar9();") and assignment to a pointer ("*cp = bar9()").

For struct return values, the patch also includes a fix to follow the
x86-64 ABI: the caller provides space for the return value in a hidden
first argument and this address is returned in %rax or %eax for a
32-bit target.  Previously the compiler was not following this rule.

Approved by: Sun Chan


Modified: trunk/osprey/be/cg/reg_live.cxx
===================================================================
--- trunk/osprey/be/cg/reg_live.cxx     2011-06-21 02:08:29 UTC (rev 3658)
+++ trunk/osprey/be/cg/reg_live.cxx     2011-06-21 04:40:48 UTC (rev 3659)
@@ -268,6 +268,17 @@
        retpreg[i] = RETURN_INFO_preg (return_info, i);
        Add_PREG_To_REGSET (retpreg[i], return_regs);
     }
+#ifdef TARG_X8664
+    // x86-64 ABI
+    // on return %rax will contain the address that has been
+    // passed in by the caller in %rdi
+    // for 32 bit, the return address is in %eax
+    if (RETURN_INFO_return_via_first_arg(return_info)) {
+       FmtAssert (RETURN_INFO_count(return_info) == 0,
+             ("Compute_Return_Regs:  more return registers than can handle"));
+       Add_PREG_To_REGSET (First_Int_Preg_Return_Offset, return_regs);
+    }
+#endif
 #if defined(TARG_SL)
     if (MTYPE_byte_size(TY_mtype(TY_ret_type(call_ty))) == 8) { //I8/U8/F8
       FmtAssert (RETURN_INFO_count(return_info) <= 1, 

Modified: trunk/osprey/be/com/wn_lower.cxx
===================================================================
--- trunk/osprey/be/com/wn_lower.cxx    2011-06-21 02:08:29 UTC (rev 3658)
+++ trunk/osprey/be/com/wn_lower.cxx    2011-06-21 04:40:48 UTC (rev 3659)
@@ -7331,7 +7331,8 @@
   ST *preg_st;
   WN *n_rhs;
   WN *wn = NULL;       // init to prevent upward-exposed use
-  RETURN_INFO return_info = Get_Return_Info(WN_ty(tree), Complex_Not_Simulated
+  RETURN_INFO return_info = Get_Return_Info(WN_ty(WN_kid0(tree)), 
+                                            Complex_Not_Simulated
 #ifdef TARG_X8664
                                            , last_call_ff2c_abi
 #endif
@@ -7353,7 +7354,7 @@
     if (WN_store_offset(tree) != 0) { // generate an ADD node for the offset
       WN *iwn = WN_CreateIntconst(OPR_INTCONST, Pointer_Mtype, MTYPE_V, 
                                  WN_store_offset(tree));
-      awn = WN_CreateExp2(OPR_ADD, Pointer_Mtype, Pointer_Mtype, awn, iwn);
+      awn = WN_CreateExp2(OPR_ADD, Pointer_Mtype, MTYPE_V, awn, iwn);
     }
     awn = lower_expr(block, awn, actions);
     WN *n_call = add_fake_parm(call, awn, WN_ty(tree));
@@ -12134,25 +12135,94 @@
       WN *n_rhs;
 
       // fix rhs
-      if (WN_operator(o_rhs) == OPR_LDID)
-        n_rhs = lower_mldid(block, o_rhs, LOWER_MLDID_MSTID);
-      else if (WN_operator(o_rhs) == OPR_ILOAD) 
-        n_rhs = lower_miload(block, o_rhs, LOWER_MLDID_MSTID);
-      else n_rhs = o_rhs;              // MLOAD
-
-      // create an mstore
+ 
       WN *first_formal = WN_formal(current_function, 0);
       TY_IDX tidx = ST_type(WN_st(first_formal));
-      WN *awn = WN_CreateLdid(OPR_LDID, 
-                             TY_mtype(Ty_Table[tidx]), 
-                             TY_mtype(Ty_Table[tidx]),
-                             WN_idname_offset(first_formal), 
-                             WN_st(first_formal), 
-                             tidx);
-      WN *swn = WN_CopyNode(WN_kid1(n_rhs));
-      wn  = WN_CreateMstore (0, tidx, n_rhs, awn, swn);
-      WN_Set_Linenum(wn, current_srcpos);  // Bug 1268
-      WN_INSERT_BlockLast (block, wn);
+      if (WN_operator(o_rhs) == OPR_LDID && 
+            WN_st(o_rhs) == Return_Val_Preg) {
+        // the Return_Val_Preg must be returned by previous call
+        // so we need to get the previous MCALL statement and 
+        // fake first parm
+        //
+        // MCALL 126 <1,51,bar9>
+        //   MMLDID -1 <1,49,.preg_return_val> T<53,.anonymous.1,1>
+        // MRETURN_VAL 
+        //
+        //  ==>
+        //
+        //     U8LDID 0 <2,3,_temp_.return...>
+        //   U8PARM 33 T<55,anon_ptr.,8>
+        // VCALL 126 <1,51,bar9>
+        // 
+        WN *call = WN_last(block);
+        if ((WN_operator(call) == OPR_CALL || WN_operator(call) == OPR_ICALL ||
+               WN_operator(call) == OPR_PICCALL) && WN_rtype(call) == MTYPE_M) 
{
+          TY_IDX prototype;
+          if (WN_operator(call) == OPR_ICALL) 
+            prototype = WN_ty(call);
+          else {
+            ST_IDX func_stidx = WN_st_idx(call);
+            PU_IDX puidx = ST_pu(St_Table[func_stidx]);
+            prototype = PU_prototype(Pu_Table[puidx]);
+          }
+          WN *awn = WN_CreateLdid(OPR_LDID, 
+                               TY_mtype(Ty_Table[tidx]), 
+                               TY_mtype(Ty_Table[tidx]),
+                               WN_idname_offset(first_formal), 
+                               WN_st(first_formal), 
+                               tidx);
+          awn = lower_expr(block, awn, actions);
+          WN *n_call = add_fake_parm(call, awn, WN_ty(awn));
+          WN_DELETE_FromBlock(block, call);
+          WN_INSERT_BlockLast(block, n_call); 
+        }
+      }
+      else {
+        if (WN_operator(o_rhs) == OPR_LDID) {
+          n_rhs = lower_mldid(block, o_rhs, LOWER_MLDID_MSTID);
+        }
+        else if (WN_operator(o_rhs) == OPR_ILOAD) 
+          n_rhs = lower_miload(block, o_rhs, LOWER_MLDID_MSTID);
+        else n_rhs = o_rhs;            // MLOAD
+  
+        // create an mstore
+        WN *awn = WN_CreateLdid(OPR_LDID, 
+                               TY_mtype(Ty_Table[tidx]), 
+                               TY_mtype(Ty_Table[tidx]),
+                               WN_idname_offset(first_formal), 
+                               WN_st(first_formal), 
+                               tidx);
+        WN *swn = WN_CopyNode(WN_kid1(n_rhs));
+        wn  = WN_CreateMstore (0, tidx, n_rhs, awn, swn);
+        WN_Set_Linenum(wn, current_srcpos);  // Bug 1268
+        WN_INSERT_BlockLast (block, wn);
+      }
+#ifdef TARG_X8664
+      // x86-64 ABI
+      // on return %rax will contain the address that has been 
+      // passed in by the caller in %rdi
+      // for 32 bit, the return address is in %eax
+      //
+      //   U8U8LDID 0 <2,3,_temp_.return...>
+      // U8STID 1 <1,5,.preg_I8>  T<5,.predef_I8,8> # $r1
+      // 
+ 
+      mtype = Is_Target_64bit() ? MTYPE_U8 : MTYPE_U4;
+      WN *ld = WN_CreateLdid(OPR_LDID, 
+                          TY_mtype(Ty_Table[tidx]), 
+                          TY_mtype(Ty_Table[tidx]),
+                          WN_idname_offset(first_formal), 
+                          WN_st(first_formal), 
+                          tidx);
+      WN *stid = WN_Stid( mtype, First_Int_Preg_Return_Offset,
+        Int_Preg, ST_type(Int_Preg), ld );
+      WN_Set_Linenum(stid, current_srcpos);
+      WN_INSERT_BlockLast( block, stid );
+      if (traceMload) {
+        fprintf(TFile, "Return_val lower [Return_Val_Preg]\n");
+        fdump_tree(TFile, block);
+      }
+#endif
     }
     else { // return in return registers
       INT32 i;
@@ -12263,6 +12333,10 @@
     }
   }
 
+  // lastly make a normal return statement
+  //
+  // RETURN 
+  //  
   WN *wn_return = WN_CreateReturn ();
   WN_Set_Linenum(wn_return, current_srcpos);  // Bug 1268
   if ( Cur_PU_Feedback )

Modified: trunk/osprey/wgen/wgen_decl.cxx
===================================================================
--- trunk/osprey/wgen/wgen_decl.cxx     2011-06-21 02:08:29 UTC (rev 3658)
+++ trunk/osprey/wgen/wgen_decl.cxx     2011-06-21 04:40:48 UTC (rev 3659)
@@ -2825,14 +2825,61 @@
         Is_True( (WN_operator(target) == OPR_LDID ||
                   WN_operator(target) == OPR_LDA),
                  ("Invalid operator for target"));
-        if( WN_operator(target) == OPR_LDID ) {
+        // handle struct init by a return value of a call,
+        // remove redundant temp variable copy
+        
+        // if return type is struct type, and the result is used in lhs,
+        // then the Mreturn temp variable is not needed, the address of lhs 
+        // should be passed as hidden parameter
+        bool return_val_transformed = false;
+        WN *block = NULL;
+        if (WN_operator(init_wn) == OPR_COMMA &&
+              WN_rtype(init_wn) == MTYPE_M) {
+           WN *block = WN_kid0(init_wn);
+           WN *ldidTemp = WN_kid1(init_wn);
+           if (WN_operator(ldidTemp) == OPR_LDID &&
+                 WN_operator(block) == OPR_BLOCK) {
+
+              // replace MSTID _temp_.Mreturn.1
+              // with    MSTID lhs
+              WN *stidTemp = WN_last(block);
+              if (WN_operator(stidTemp) == OPR_STID) {
+                 // remove block from the init_wn
+                 WN_kid0(init_wn) = 0;
+                 WN_DELETE_Tree(init_wn);
+                 init_wn = block;
+                 
+                 WN *kid = WN_kid0(stidTemp);
+                 WN_kid0(stidTemp) = 0;
+                 WN_DELETE_FromBlock(block, stidTemp);
+
+                 if( WN_operator(target) == OPR_LDID ) {
+                   TY_IDX ptr_ty = Make_Pointer_Type(ty);
+                   wn = WN_Istore(mtype, offset, ptr_ty, target, kid, 
field_id);
+                 }
+                 else { // OPR_LDA
+                   ST *st = WN_st(target);
+                   wn = WN_Stid (mtype, WN_lda_offset(target) + offset, st,
+                                 ty, kid, field_id); 
+                 }
+                 WN_INSERT_BlockLast(block, wn);
+                 wn = block;
+                 return_val_transformed = true;
+              }
+           }
+           else
+              block = NULL; // not a block
+        }
+        if (!return_val_transformed) {
+          if( WN_operator(target) == OPR_LDID ) {
             TY_IDX ptr_ty = Make_Pointer_Type(ty);
             wn = WN_Istore(mtype, offset, ptr_ty, target, init_wn, field_id);
-        }
-        else { // OPR_LDA
+          }
+          else { // OPR_LDA
             ST *st = WN_st(target);
             wn = WN_Stid (mtype, WN_lda_offset(target) + offset, st,
                           ty, init_wn, field_id); 
+          }
         }
 #else
        WN *wn = WN_Stid (mtype, ST_ofst(st) + offset, st,

Modified: trunk/osprey/wgen/wgen_expr.cxx
===================================================================
--- trunk/osprey/wgen/wgen_expr.cxx     2011-06-21 02:08:29 UTC (rev 3658)
+++ trunk/osprey/wgen/wgen_expr.cxx     2011-06-21 04:40:48 UTC (rev 3659)
@@ -1645,9 +1645,51 @@
        if (volt) 
          Set_TY_is_volatile(hi_ty_idx);
 #endif
-        wn = WN_Stid (desc, ST_ofst(st) + component_offset + lhs_preg_num, st,
-                     hi_ty_idx, rhs_wn, field_id);
-        WGEN_Stmt_Append(wn, Get_Srcpos());
+        // if return type is struct type, and the result is used in lhs,
+        // then the Mreturn temp variable is not needed, the address of lhs 
+        // should be passed as hidden parameter
+
+        // the same pattern match is used in different places
+        // to handle return struct value, init struct value and
+        // assign struct value from a function call, if anything
+        // changed in the pattern in future, we need to change all 
+        // these places, just look for return_val_transformed.
+        bool return_val_transformed =  false;
+        if (WN_operator(rhs_wn) == OPR_COMMA &&
+              WN_rtype(rhs_wn) == MTYPE_M) {
+           WN *block = WN_kid0(rhs_wn);
+           WN *ldidTemp = WN_kid1(rhs_wn);
+           if (WN_operator(ldidTemp) == OPR_LDID &&
+                 WN_operator(block) == OPR_BLOCK) {
+
+              // replace MSTID _temp_.Mreturn.1
+              // with    MSTID lhs
+              WN *stidTemp = WN_last(block);
+              if (WN_operator(stidTemp) == OPR_STID ) {
+                 // remove block from the rhs_wn
+                 WN_kid0(rhs_wn) = 0;
+                 WN_DELETE_Tree(rhs_wn);
+                 rhs_wn = block;
+
+                 WN *kid = WN_kid0(stidTemp);
+                 WN_kid0(stidTemp) = 0;
+                 WN_DELETE_FromBlock(block, stidTemp);
+
+                 WN *stid = WN_Stid (desc, 
+                       ST_ofst(st) + component_offset + lhs_preg_num,
+                       st, hi_ty_idx, kid, field_id);
+                 WN_INSERT_BlockLast(block, stid);
+                 WGEN_Stmt_Append(block, Get_Srcpos());
+                 wn = block;
+                 return_val_transformed = true;
+              }
+           }
+        }
+        if (!return_val_transformed) {
+           wn = WN_Stid (desc, ST_ofst(st) + component_offset + lhs_preg_num, 
st,
+                        hi_ty_idx, rhs_wn, field_id);
+           WGEN_Stmt_Append(wn, Get_Srcpos());
+        }
 #if defined(TARG_SL)
         if (need_append) {
           WN *ldid_wn;
@@ -1869,7 +1911,6 @@
         wn = NULL;
       }
       else {
-#ifdef KEY
        // The store target could be an INDIRECT_REF that kg++fe added to make
        // the store write to the area pointed to by the fake first param.  If
        // so, check that copying the object does not involve a copy
@@ -1889,31 +1930,66 @@
         if (Current_Entry_WN() != NULL) {
             first_formal = WN_formal(Current_Entry_WN(), 0);
         }
-        if (TY_return_in_mem(hi_ty_idx) &&
-           field_id == 0 &&
-           // See if it is an indirect ref of the fake first parm.
-           // bug fix for OSP_314
-           //
-           first_formal != NULL && (WN_operator(first_formal) != OPR_BLOCK) &&
-           gs_tree_code(addr) == GS_VAR_DECL &&
-           DECL_ST(addr) == WN_st(first_formal)) {
-         FmtAssert(TY_mtype(hi_ty_idx) == MTYPE_M,
-                   ("WGEN_Lhs_Of_Modify_Expr: return_in_mem type not 
MTYPE_M"));
-         gs_t ptr_type = gs_tree_type(gs_tree_operand(lhs, 0));
-         gs_t type = gs_tree_type(ptr_type);
-         FmtAssert(gs_tree_code(ptr_type) == GS_POINTER_TYPE,
-           ("WGEN_Lhs_Of_Modify_Expr: INDIRECT_REF opnd0 is not 
POINTER_TYPE"));
-         FmtAssert(component_offset == 0,
-                   ("WGEN_Lhs_Of_Modify_Expr: component_offset nonzero"));
-         TY_IDX tidx = Get_TY(ptr_type);
-         // Check object has no copy constructor.
-         FmtAssert(!WGEN_has_copy_constructor(type),
-             ("WGEN_Lhs_Of_Modify_Expr: object needs copy constructor"));
+        // if return type is struct type, and the result is used in lhs,
+        // then the Mreturn temp variable is not needed, the address of lhs 
+        // should be passed as hidden parameter
+        bool return_val_transformed = false;
+        if (WN_operator(rhs_wn) == OPR_COMMA &&
+              WN_rtype(rhs_wn) == MTYPE_M) {
+           WN *block = WN_kid0(rhs_wn);
+           WN *ldidTemp = WN_kid1(rhs_wn);
+           if (WN_operator(ldidTemp) == OPR_LDID &&
+                 WN_operator(block) == OPR_BLOCK) {
+
+              // replace MSTID _temp_.Mreturn.1
+              // with    MISTORE lhs
+              WN *stidTemp = WN_last(block);
+              if (WN_operator(stidTemp) == OPR_STID) {
+                 // remove block from the rhs_wn
+                 WN_kid0(rhs_wn) = 0;
+                 WN_DELETE_Tree(rhs_wn);
+                 rhs_wn = block;
+
+                 WN *kid = WN_kid0(stidTemp);
+                 WN_kid0(stidTemp) = 0;
+                 WN_DELETE_FromBlock(block, stidTemp);
+
+                 WN *istore = 
+                    WN_CreateIstore(OPR_ISTORE, MTYPE_V, desc, 
component_offset, 
+                              Make_Pointer_Type (hi_ty_idx, FALSE),
+                              kid, addr_wn, field_id);
+                 WN_INSERT_BlockLast(block, istore);
+                 wn = block;
+                 return_val_transformed = true;
+              }
+           }
         }
-#endif
-        wn = WN_CreateIstore(OPR_ISTORE, MTYPE_V, desc, component_offset, 
-                            Make_Pointer_Type (hi_ty_idx, FALSE),
-                            rhs_wn, addr_wn, field_id);
+        if (!return_val_transformed) {
+          if (TY_return_in_mem(hi_ty_idx) &&
+             field_id == 0 &&
+             // See if it is an indirect ref of the fake first parm.
+             // bug fix for OSP_314
+             //
+             first_formal != NULL && (WN_operator(first_formal) != OPR_BLOCK) 
&&
+             gs_tree_code(addr) == GS_VAR_DECL &&
+             DECL_ST(addr) == WN_st(first_formal)) {
+           FmtAssert(TY_mtype(hi_ty_idx) == MTYPE_M,
+                     ("WGEN_Lhs_Of_Modify_Expr: return_in_mem type not 
MTYPE_M"));
+           gs_t ptr_type = gs_tree_type(gs_tree_operand(lhs, 0));
+           gs_t type = gs_tree_type(ptr_type);
+           FmtAssert(gs_tree_code(ptr_type) == GS_POINTER_TYPE,
+             ("WGEN_Lhs_Of_Modify_Expr: INDIRECT_REF opnd0 is not 
POINTER_TYPE"));
+           FmtAssert(component_offset == 0,
+                     ("WGEN_Lhs_Of_Modify_Expr: component_offset nonzero"));
+           TY_IDX tidx = Get_TY(ptr_type);
+           // Check object has no copy constructor.
+           FmtAssert(!WGEN_has_copy_constructor(type),
+               ("WGEN_Lhs_Of_Modify_Expr: object needs copy constructor"));
+          }
+          wn = WN_CreateIstore(OPR_ISTORE, MTYPE_V, desc, component_offset, 
+                              Make_Pointer_Type (hi_ty_idx, FALSE),
+                              rhs_wn, addr_wn, field_id);
+        }
 #ifdef TARG_SL
         /* so far I only handle *p++=... cases, change this case to 
          *   *p = ... ;

Modified: trunk/osprey/wgen/wgen_spin_symbol.cxx
===================================================================
--- trunk/osprey/wgen/wgen_spin_symbol.cxx      2011-06-21 02:08:29 UTC (rev 
3658)
+++ trunk/osprey/wgen/wgen_spin_symbol.cxx      2011-06-21 04:40:48 UTC (rev 
3659)
@@ -852,6 +852,7 @@
                        Set_TY_is_union(idx);
                }
 #ifdef KEY
+                // gs_aggregate_value_p is only set for c++
                if (gs_aggregate_value_p(type_tree)) {
                        Set_TY_return_in_mem(idx);
                }


------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to