[PATCH 1/2] Add new target hook: constant_ok_for_cprop_p

[PATCH 1/2] Add new target hook: constant_ok_for_cprop_p

[Takayuki 'January June' Suwa]

Hi,

Many RISC machines, as we know, have some restrictions on placing register-width constants in the source of load-immediate machine instructions, so the target must provide a solution for that in the machine description.

A naive way would be to solve it early, ie. to replace with read constants pooled in memory when expanding to RTL.

Alternatively, a more fancy approach would be to forgo placement in the constant pool until somewhere before the reload/LRA eg. the "split1" pass to give the optimization passes that involve immediates a chance to work.

If we choose the latter, we can expect better results with RTL if-conversion, constant folding, etc., but it often propagates constants that are too large in size to resolve to a simple load-immediate instruction.

This is because constant propagation has no way of telling about it, so this patch provides it.

===

This new target hook can be used to tell cprop whether or not to propagate
a constant depending on its contents.

For backwards compatibility, the default setting for this hook retains the
old behavior.

gcc/ChangeLog:

	* hooks.h (hook_bool_const_rtx_true): New prototype.
	* hooks.cc (hook_bool_const_rtx_true): New default hook.
	* target.def (constant_ok_for_cprop_p): New target hook.
	* cprop.cc (cprop_constant_p): Change to use the hook.
	* doc/tm.texi.in, (TARGET_CONSTANT_OK_FOR_CPROP_P): New @hook.
	* doc/tm.texi (TARGET_CONSTANT_OK_FOR_CPROP_P): New document.
---
 gcc/cprop.cc       |  4 +++-
 gcc/doc/tm.texi    | 12 ++++++++++++
 gcc/doc/tm.texi.in |  2 ++
 gcc/hooks.cc       |  7 +++++++
 gcc/hooks.h        |  1 +
 gcc/target.def     | 14 ++++++++++++++
 6 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/gcc/cprop.cc b/gcc/cprop.cc
index 580f811545d..dfb1e88e9b4 100644
--- a/gcc/cprop.cc
+++ b/gcc/cprop.cc
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "dbgcnt.h"
 #include "cfgloop.h"
 #include "gcse.h"
+#include "target.h"
 
 \f
 /* An obstack for our working variables.  */
@@ -249,7 +250,8 @@ insert_set_in_table (rtx dest, rtx src, rtx_insn *insn,
 static bool
 cprop_constant_p (const_rtx x)
 {
-  return CONSTANT_P (x) && (GET_CODE (x) != CONST || shared_const_p (x));
+  return CONSTANT_P (x) && targetm.constant_ok_for_cprop_p (x)
+	 && (GET_CODE (x) != CONST || shared_const_p (x));
 }
 
 /* Determine whether the rtx X should be treated as a register that can
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 858bfb80cec..83151626a71 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12187,6 +12187,18 @@ MIPS, where add-immediate takes a 16-bit signed value,
 is zero, which disables this optimization.
 @end deftypevr
 
+@deftypefn {Target Hook} bool TARGET_CONSTANT_OK_FOR_CPROP_P (const_rtx @var{cst})
+On some target machines, such as RISC ones, load-immediate instructions
+often have a limited range (for example, within signed 12 bits or less).
+Because they will be typically placed into the constant pool,
+unconditionally propagating constants that exceed such limit can lead to
+increased number of instruction and/or memory read access.
+This target hook should return @code{false} if @var{cst}, a candidate for
+constant propagation, is undesirable as a source for load-immediate
+instructions.
+The default version of this hook always returns @code{true}.
+@end deftypefn
+
 @deftypefn {Target Hook} {unsigned HOST_WIDE_INT} TARGET_ASAN_SHADOW_OFFSET (void)
 Return the offset bitwise ored into shifted address to get corresponding
 Address Sanitizer shadow memory address.  NULL if Address Sanitizer is not
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 21b849ea32a..147331b0f53 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7887,6 +7887,8 @@ and the associated definitions of those functions.
 
 @hook TARGET_CONST_ANCHOR
 
+@hook TARGET_CONSTANT_OK_FOR_CPROP_P
+
 @hook TARGET_ASAN_SHADOW_OFFSET
 
 @hook TARGET_MEMMODEL_CHECK
diff --git a/gcc/hooks.cc b/gcc/hooks.cc
index b29233f4f85..67bf3553d26 100644
--- a/gcc/hooks.cc
+++ b/gcc/hooks.cc
@@ -82,6 +82,13 @@ hook_bool_mode_true (machine_mode)
   return true;
 }
 
+/* Generic hook that takes (const_rtx) and returns true.  */
+bool
+hook_bool_const_rtx_true (const_rtx)
+{
+  return true;
+}
+
 /* Generic hook that takes (machine_mode, machine_mode) and returns true.  */
 bool
 hook_bool_mode_mode_true (machine_mode, machine_mode)
diff --git a/gcc/hooks.h b/gcc/hooks.h
index 1056e1e9e4d..d001f8fb9dc 100644
--- a/gcc/hooks.h
+++ b/gcc/hooks.h
@@ -30,6 +30,7 @@ extern bool hook_bool_bool_gcc_optionsp_false (bool, struct gcc_options *);
 extern bool hook_bool_const_int_const_int_true (const int, const int);
 extern bool hook_bool_mode_false (machine_mode);
 extern bool hook_bool_mode_true (machine_mode);
+extern bool hook_bool_const_rtx_true (const_rtx);
 extern bool hook_bool_mode_mode_true (machine_mode, machine_mode);
 extern bool hook_bool_mode_const_rtx_false (machine_mode, const_rtx);
 extern bool hook_bool_mode_const_rtx_true (machine_mode, const_rtx);
diff --git a/gcc/target.def b/gcc/target.def
index 4d49ffc2c88..8bf093ab0dc 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -4510,6 +4510,20 @@ MIPS, where add-immediate takes a 16-bit signed value,\n\
 is zero, which disables this optimization.",
  unsigned HOST_WIDE_INT, 0)
 
+DEFHOOK
+(constant_ok_for_cprop_p,
+ "On some target machines, such as RISC ones, load-immediate instructions\n\
+often have a limited range (for example, within signed 12 bits or less).\n\
+Because they will be typically placed into the constant pool,\n\
+unconditionally propagating constants that exceed such limit can lead to\n\
+increased number of instruction and/or memory read access.\n\
+This target hook should return @code{false} if @var{cst}, a candidate for\n\
+constant propagation, is undesirable as a source for load-immediate\n\
+instructions.\n\
+The default version of this hook always returns @code{true}.",
+ bool, (const_rtx cst),
+ hook_bool_const_rtx_true)
+
 /* Defines, which target-dependent bits (upper 16) are used by port  */
 DEFHOOK
 (memmodel_check,
-- 
2.20.1

Re: [PATCH 1/2] Add new target hook: constant_ok_for_cprop_p

[Richard Biener]

On Sun, Sep 11, 2022 at 10:51 PM Takayuki 'January June' Suwa via
Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> Many RISC machines, as we know, have some restrictions on placing register-width constants in the source of load-immediate machine instructions, so the target must provide a solution for that in the machine description.
>
> A naive way would be to solve it early, ie. to replace with read constants pooled in memory when expanding to RTL.
>
> Alternatively, a more fancy approach would be to forgo placement in the constant pool until somewhere before the reload/LRA eg. the "split1" pass to give the optimization passes that involve immediates a chance to work.
>
> If we choose the latter, we can expect better results with RTL if-conversion, constant folding, etc., but it often propagates constants that are too large in size to resolve to a simple load-immediate instruction.
>
> This is because constant propagation has no way of telling about it, so this patch provides it.

What does prevent other passes like fwprop, CSE and PRE from doing the
same propagation?  Can that be the solution for
constant propagation as well?

Richard.

> ===
>
> This new target hook can be used to tell cprop whether or not to propagate
> a constant depending on its contents.
>
> For backwards compatibility, the default setting for this hook retains the
> old behavior.
>
> gcc/ChangeLog:
>
>         * hooks.h (hook_bool_const_rtx_true): New prototype.
>         * hooks.cc (hook_bool_const_rtx_true): New default hook.
>         * target.def (constant_ok_for_cprop_p): New target hook.
>         * cprop.cc (cprop_constant_p): Change to use the hook.
>         * doc/tm.texi.in, (TARGET_CONSTANT_OK_FOR_CPROP_P): New @hook.
>         * doc/tm.texi (TARGET_CONSTANT_OK_FOR_CPROP_P): New document.
> ---
>  gcc/cprop.cc       |  4 +++-
>  gcc/doc/tm.texi    | 12 ++++++++++++
>  gcc/doc/tm.texi.in |  2 ++
>  gcc/hooks.cc       |  7 +++++++
>  gcc/hooks.h        |  1 +
>  gcc/target.def     | 14 ++++++++++++++
>  6 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/cprop.cc b/gcc/cprop.cc
> index 580f811545d..dfb1e88e9b4 100644
> --- a/gcc/cprop.cc
> +++ b/gcc/cprop.cc
> @@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "dbgcnt.h"
>  #include "cfgloop.h"
>  #include "gcse.h"
> +#include "target.h"
>
>
>  /* An obstack for our working variables.  */
> @@ -249,7 +250,8 @@ insert_set_in_table (rtx dest, rtx src, rtx_insn *insn,
>  static bool
>  cprop_constant_p (const_rtx x)
>  {
> -  return CONSTANT_P (x) && (GET_CODE (x) != CONST || shared_const_p (x));
> +  return CONSTANT_P (x) && targetm.constant_ok_for_cprop_p (x)
> +        && (GET_CODE (x) != CONST || shared_const_p (x));
>  }
>
>  /* Determine whether the rtx X should be treated as a register that can
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 858bfb80cec..83151626a71 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -12187,6 +12187,18 @@ MIPS, where add-immediate takes a 16-bit signed value,
>  is zero, which disables this optimization.
>  @end deftypevr
>
> +@deftypefn {Target Hook} bool TARGET_CONSTANT_OK_FOR_CPROP_P (const_rtx @var{cst})
> +On some target machines, such as RISC ones, load-immediate instructions
> +often have a limited range (for example, within signed 12 bits or less).
> +Because they will be typically placed into the constant pool,
> +unconditionally propagating constants that exceed such limit can lead to
> +increased number of instruction and/or memory read access.
> +This target hook should return @code{false} if @var{cst}, a candidate for
> +constant propagation, is undesirable as a source for load-immediate
> +instructions.
> +The default version of this hook always returns @code{true}.
> +@end deftypefn
> +
>  @deftypefn {Target Hook} {unsigned HOST_WIDE_INT} TARGET_ASAN_SHADOW_OFFSET (void)
>  Return the offset bitwise ored into shifted address to get corresponding
>  Address Sanitizer shadow memory address.  NULL if Address Sanitizer is not
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 21b849ea32a..147331b0f53 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -7887,6 +7887,8 @@ and the associated definitions of those functions.
>
>  @hook TARGET_CONST_ANCHOR
>
> +@hook TARGET_CONSTANT_OK_FOR_CPROP_P
> +
>  @hook TARGET_ASAN_SHADOW_OFFSET
>
>  @hook TARGET_MEMMODEL_CHECK
> diff --git a/gcc/hooks.cc b/gcc/hooks.cc
> index b29233f4f85..67bf3553d26 100644
> --- a/gcc/hooks.cc
> +++ b/gcc/hooks.cc
> @@ -82,6 +82,13 @@ hook_bool_mode_true (machine_mode)
>    return true;
>  }
>
> +/* Generic hook that takes (const_rtx) and returns true.  */
> +bool
> +hook_bool_const_rtx_true (const_rtx)
> +{
> +  return true;
> +}
> +
>  /* Generic hook that takes (machine_mode, machine_mode) and returns true.  */
>  bool
>  hook_bool_mode_mode_true (machine_mode, machine_mode)
> diff --git a/gcc/hooks.h b/gcc/hooks.h
> index 1056e1e9e4d..d001f8fb9dc 100644
> --- a/gcc/hooks.h
> +++ b/gcc/hooks.h
> @@ -30,6 +30,7 @@ extern bool hook_bool_bool_gcc_optionsp_false (bool, struct gcc_options *);
>  extern bool hook_bool_const_int_const_int_true (const int, const int);
>  extern bool hook_bool_mode_false (machine_mode);
>  extern bool hook_bool_mode_true (machine_mode);
> +extern bool hook_bool_const_rtx_true (const_rtx);
>  extern bool hook_bool_mode_mode_true (machine_mode, machine_mode);
>  extern bool hook_bool_mode_const_rtx_false (machine_mode, const_rtx);
>  extern bool hook_bool_mode_const_rtx_true (machine_mode, const_rtx);
> diff --git a/gcc/target.def b/gcc/target.def
> index 4d49ffc2c88..8bf093ab0dc 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -4510,6 +4510,20 @@ MIPS, where add-immediate takes a 16-bit signed value,\n\
>  is zero, which disables this optimization.",
>   unsigned HOST_WIDE_INT, 0)
>
> +DEFHOOK
> +(constant_ok_for_cprop_p,
> + "On some target machines, such as RISC ones, load-immediate instructions\n\
> +often have a limited range (for example, within signed 12 bits or less).\n\
> +Because they will be typically placed into the constant pool,\n\
> +unconditionally propagating constants that exceed such limit can lead to\n\
> +increased number of instruction and/or memory read access.\n\
> +This target hook should return @code{false} if @var{cst}, a candidate for\n\
> +constant propagation, is undesirable as a source for load-immediate\n\
> +instructions.\n\
> +The default version of this hook always returns @code{true}.",
> + bool, (const_rtx cst),
> + hook_bool_const_rtx_true)
> +
>  /* Defines, which target-dependent bits (upper 16) are used by port  */
>  DEFHOOK
>  (memmodel_check,
> --
> 2.20.1

Re: [PATCH 1/2] Add new target hook: constant_ok_for_cprop_p

[Jeff Law]

On 9/12/22 01:35, Richard Biener via Gcc-patches wrote:
> On Sun, Sep 11, 2022 at 10:51 PM Takayuki 'January June' Suwa via
> Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> Hi,
>>
>> Many RISC machines, as we know, have some restrictions on placing register-width constants in the source of load-immediate machine instructions, so the target must provide a solution for that in the machine description.
>>
>> A naive way would be to solve it early, ie. to replace with read constants pooled in memory when expanding to RTL.
>>
>> Alternatively, a more fancy approach would be to forgo placement in the constant pool until somewhere before the reload/LRA eg. the "split1" pass to give the optimization passes that involve immediates a chance to work.
>>
>> If we choose the latter, we can expect better results with RTL if-conversion, constant folding, etc., but it often propagates constants that are too large in size to resolve to a simple load-immediate instruction.
>>
>> This is because constant propagation has no way of telling about it, so this patch provides it.
> What does prevent other passes like fwprop, CSE and PRE from doing the
> same propagation?  Can that be the solution for
> constant propagation as well?

I would think this should be driven by costing rather than a new hook.  
I'm pretty sure we already use costing to determine some of this stuff 
for CSE/PRE.


jeff