Re: [PATCH V2] Use subscalar mode to move struct block for parameter

[Jeff Law <jeffreyalaw@gmail.com>]

On 11/20/22 20:07, Jiufu Guo wrote:
> Jiufu Guo <guojiufu@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?


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.


Jeff