diff mbox series

internal-fn: Convert uninitialized SSA_NAME into SCRATCH rtx[PR110751]

Message ID 20230916044539.3362257-1-juzhe.zhong@rivai.ai
State New
Headers
Series internal-fn: Convert uninitialized SSA_NAME into SCRATCH rtx[PR110751] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed

Commit Message

钟居哲 Sept. 16, 2023, 4:45 a.m. UTC 
  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(+)

Comments

Richard Sandiford Sept. 17, 2023, 10:29 a.m. UTC | #1 
Juzhe-Zhong <juzhe.zhong@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;
钟居哲 Sept. 17, 2023, 2:49 p.m. UTC | #2 
Thanks Richard.

Address comment in V2:
[PATCH V2] internal-fn: Support undefined rtx for uninitialized SSA_NAME (gnu.org)




juzhe.zhong@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.zhong@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;
diff mbox series

Patch

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))
+	create_input_operand (&ops[opno], scratch, TYPE_MODE (rhs_type));
       else
 	create_input_operand (&ops[opno], rhs_rtx, TYPE_MODE (rhs_type));
       opno += 1;