Thanks Richard. Address comment in V2: [PATCH V2] internal-fn: Support undefined rtx for uninitialized SSA_NAME (gnu.org)
juzhe.zh...@rivai.ai From: Richard Sandiford Date: 2023-09-17 18:29 To: Juzhe-Zhong CC: gcc-patches; rguenther Subject: Re: [PATCH] internal-fn: Convert uninitialized SSA_NAME into SCRATCH rtx[PR110751] Juzhe-Zhong <juzhe.zh...@rivai.ai> writes: > According to PR: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110751 > > As Richard and Richi suggested, we recognize uninitialized SSA_NAME and > convert it > into SCRATCH rtx if the target predicate allows SCRATCH. > > It can help to reduce redundant data move instructions of targets like RISC-V. > > Here we add the condition "insn_operand_matches (icode, opno, scratch)" > Then, we will only create scratch rtx that target allow scratch rtx in > predicate. > When the target doesn't allow scratch rtx in predicate, the later "else" > condtion > will create fresh pseudo for uninitialized SSA. > > I have verify it in RISC-V port and it works well. > > Bootstrap and Regression on X86 passed. > > Ok for trunk ? > > gcc/ChangeLog: > > * internal-fn.cc (expand_fn_using_insn): Convert uninitialized SSA into > scratch. > > --- > gcc/internal-fn.cc | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc > index 0fd34359247..fe4d86b3dbd 100644 > --- a/gcc/internal-fn.cc > +++ b/gcc/internal-fn.cc > @@ -243,10 +243,16 @@ expand_fn_using_insn (gcall *stmt, insn_code icode, > unsigned int noutputs, > tree rhs = gimple_call_arg (stmt, i); > tree rhs_type = TREE_TYPE (rhs); > rtx rhs_rtx = expand_normal (rhs); > + rtx scratch = gen_rtx_SCRATCH (TYPE_MODE (rhs_type)); > if (INTEGRAL_TYPE_P (rhs_type)) > create_convert_operand_from (&ops[opno], rhs_rtx, > TYPE_MODE (rhs_type), > TYPE_UNSIGNED (rhs_type)); > + else if (TREE_CODE (rhs) == SSA_NAME > + && SSA_NAME_IS_DEFAULT_DEF (rhs) > + && VAR_P (SSA_NAME_VAR (rhs)) > + && insn_operand_matches (icode, opno, scratch)) Rather than check insn_operand_matches here, I think we should create the scratch operand regardless and leave optabs.cc to deal with it. (This will need changes to optabs.cc.) How about adding: create_undefined_input_operand (expand_operand *op, machine_mode mode) that maps to a new EXPAND_UNDEFINED, then handle EXPAND_UNDEFINED in the two case statements in optabs.cc. Thanks, Richard > + create_input_operand (&ops[opno], scratch, TYPE_MODE (rhs_type)); > else > create_input_operand (&ops[opno], rhs_rtx, TYPE_MODE (rhs_type)); > opno += 1;