On 11/22/22 19:58, Jiufu Guo wrote:
Hi Jeff,

Thanks a lot for your comments!

Jeff Law <jeffreya...@gmail.com> writes:

On 11/20/22 20:07, Jiufu Guo wrote:
Jiufu Guo <guoji...@linux.ibm.com> writes:

Hi,

As mentioned in the previous version patch:
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604646.html
The suboptimal code is generated for "assigning from parameter" or
"assigning to return value".
This patch enhances the assignment from parameters like the below
cases:
/////case1.c
typedef struct SA {double a[3];long l; } A;
A ret_arg (A a) {return a;}
void st_arg (A a, A *p) {*p = a;}

////case2.c
typedef struct SA {double a[3];} A;
A ret_arg (A a) {return a;}
void st_arg (A a, A *p) {*p = a;}

For this patch, bootstrap and regtest pass on ppc64{,le}
and x86_64.
* Besides asking for help reviewing this patch, I would like to
consult comments about enhancing for "assigning to returns".
I updated the patch to fix the issue for returns.  This patch
adds a flag DECL_USEDBY_RETURN_P to indicate if a var is used
by a return stmt.  This patch fix the issue in expand pass only,
so, we would try to update the patch to avoid this flag.

diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index dd29ffffc03..09b8ec64cea 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -2158,6 +2158,20 @@ expand_used_vars (bitmap forced_stack_vars)
       frame_phase = off ? align - off : 0;
     }
   +  /* Collect VARs on returns.  */
+  if (DECL_RESULT (current_function_decl))
+    {
+      edge_iterator ei;
+      edge e;
+      FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds)
+       if (greturn *ret = safe_dyn_cast<greturn *> (last_stmt (e->src)))
+         {
+           tree val = gimple_return_retval (ret);
+           if (val && VAR_P (val))
+             DECL_USEDBY_RETURN_P (val) = 1;
+         }
+    }
+
     /* Set TREE_USED on all variables in the local_decls.  */
     FOR_EACH_LOCAL_DECL (cfun, i, var)
       TREE_USED (var) = 1;
diff --git a/gcc/expr.cc b/gcc/expr.cc
index d9407432ea5..20973649963 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -6045,6 +6045,52 @@ expand_assignment (tree to, tree from, bool nontemporal)
         return;
       }
   +  if ((TREE_CODE (from) == PARM_DECL && DECL_INCOMING_RTL (from)
+       && TYPE_MODE (TREE_TYPE (from)) == BLKmode
+       && (GET_CODE (DECL_INCOMING_RTL (from)) == PARALLEL
+          || REG_P (DECL_INCOMING_RTL (from))))
+      || (VAR_P (to) && DECL_USEDBY_RETURN_P (to)
+         && TYPE_MODE (TREE_TYPE (to)) == BLKmode
+         && GET_CODE (DECL_RTL (DECL_RESULT (current_function_decl)))
+              == PARALLEL))
+    {
+      push_temp_slots ();
+      rtx par_ret;
+      machine_mode mode;
+      par_ret = TREE_CODE (from) == PARM_DECL
+                 ? DECL_INCOMING_RTL (from)
+                 : DECL_RTL (DECL_RESULT (current_function_decl));
+      mode = GET_CODE (par_ret) == PARALLEL
+              ? GET_MODE (XEXP (XVECEXP (par_ret, 0, 0), 0))
+              : word_mode;
+      int mode_size = GET_MODE_SIZE (mode).to_constant ();
+      int size = INTVAL (expr_size (from));
+
+      /* If/How the parameter using submode, it dependes on the size and
+        position of the parameter.  Here using heurisitic number.  */
+      int hurstc_num = 8;
Where did this come from and what does it mean?
Sorry for does not make this clear. We know that an aggregate arg may be
on registers partially or totally, as assign_parm_adjust_entry_rtl.
For an example, if a parameter with 12 words and the target/ABI only
allow 8 gprs for arguments, then the parameter could use 8 regs at most
and left part in stack.

Right, but the number of registers is target dependent, so I don't see how using "8" or any number of that matter is correct here.




Note that BLKmode subword values passed in registers can be either
right or left justified.  I think you also need to worry about
endianness here.
Since the subword is used to move block(read from source mem and then
store to destination mem with register mode), and this would keep to use
the same endianness on reg like move_block_from_reg. So, the patch does
not check the endianness.

Hmm, I was clear enough here, particularly using the endianness term.  Don't you need to query the ABI to ensure that you're not changing left vs right justification for a partially in register argument.     On the PA we have:

/* Specify padding for the last element of a block move between registers
   and memory.

   The 64-bit runtime specifies that objects need to be left justified
   (i.e., the normal justification for a big endian target).  The 32-bit
   runtime specifies right justification for objects smaller than 64 bits.
   We use a DImode register in the parallel for 5 to 7 byte structures
   so that there is only one element.  This allows the object to be
   correctly padded.  */
#define BLOCK_REG_PADDING(MODE, TYPE, FIRST) \
  targetm.calls.function_arg_padding ((MODE), (TYPE))


Jeff




Reply via email to