public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Add targetm.have_ccmp hook [PR115370]
@ 2024-06-13  2:43 Hongyu Wang
  2024-06-13  9:03 ` Richard Sandiford
  0 siblings, 1 reply; 3+ messages in thread
From: Hongyu Wang @ 2024-06-13  2:43 UTC (permalink / raw)
  To: gcc-patches; +Cc: hongtao.liu, ubizjak, richard.guenther, richard.sandiford

Hi,

In cfgexpand, there is an optimization for branch which tests
targetm.gen_ccmp_first == NULL. However for target like x86-64, the
hook was implemented but it does not indicate that ccmp was enabled.
Add a new target hook TARGET_HAVE_CCMP and replace the middle-end
check for the existance of gen_ccmp_first to avoid misoptimization.

This fixes PR115370 that have suboptimal codegen, also I checked the
znver2 binary for 526 and it will have same binary as the one before
the CCMP support patch r15-1058, so suppose it could also fix PR115463.

Bootstrapped/regtested on x86-64-pc-linux-gnu and aarch64-none-linux-gnu.

Ok for trunk?

gcc/ChangeLog:

	PR target/115370
	PR target/115463
	* cfgexpand.cc (expand_gimple_cond): Call targetm.have_ccmp
	instead of checking if targetm.gen_ccmp_first exists.
	* expr.cc (expand_expr_real_gassign): Likewise.
	* config/i386/i386.cc (ix86_have_ccmp): New target hook to
	check if APX_CCMP enabled.
	(TARGET_HAVE_CCMP): Define.
	* doc/tm.texi: Add TARGET_HAVE_CCMP.
	* doc/tm.texi.in: Regenerated.
	* target.def (TARGET_HAVE_CCMP): New target hook.
	* targhooks.cc (default_have_ccmp): New function.
	* targhooks.h (default_have_ccmp): New prototype.
---
 gcc/cfgexpand.cc        | 2 +-
 gcc/config/i386/i386.cc | 9 +++++++++
 gcc/doc/tm.texi         | 6 ++++++
 gcc/doc/tm.texi.in      | 2 ++
 gcc/expr.cc             | 2 +-
 gcc/target.def          | 9 +++++++++
 gcc/targhooks.cc        | 6 ++++++
 gcc/targhooks.h         | 1 +
 8 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 8de5f2ba58b..dad3ae1b7c6 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -2646,7 +2646,7 @@ expand_gimple_cond (basic_block bb, gcond *stmt)
 	  /* If jumps are cheap and the target does not support conditional
 	     compare, turn some more codes into jumpy sequences.  */
 	  else if (BRANCH_COST (optimize_insn_for_speed_p (), false) < 4
-		   && targetm.gen_ccmp_first == NULL)
+		   && !targetm.have_ccmp ())
 	    {
 	      if ((code2 == BIT_AND_EXPR
 		   && TYPE_PRECISION (TREE_TYPE (op0)) == 1
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index 173db213d14..c72f64da983 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -26204,6 +26204,13 @@ ix86_memtag_add_tag (rtx base, poly_int64 offset, unsigned char tag_offset)
   return plus_constant (Pmode, tagged_addr, offset);
 }
 
+/* Implement TARGET_HAVE_CCMP.  */
+static bool
+ix86_have_ccmp ()
+{
+  return (bool) TARGET_APX_CCMP;
+}
+
 /* Target-specific selftests.  */
 
 #if CHECKING_P
@@ -27043,6 +27050,8 @@ ix86_libgcc_floating_mode_supported_p
 #undef TARGET_GEN_CCMP_NEXT
 #define TARGET_GEN_CCMP_NEXT ix86_gen_ccmp_next
 
+#undef TARGET_HAVE_CCMP
+#define TARGET_HAVE_CCMP ix86_have_ccmp
 
 static bool
 ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED)
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 8a7aa70d605..993816deeba 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12354,6 +12354,12 @@ This function prepares to emit a conditional comparison within a sequence
  @var{bit_code} is @code{AND} or @code{IOR}, which is the op on the compares.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_HAVE_CCMP (void)
+This target hook returns true if the target supports conditional compare.
+This target hook is required only when the target has conditional compare that
+was not enabled by default, such as x86-64.
+@end deftypefn
+
 @deftypefn {Target Hook} unsigned TARGET_LOOP_UNROLL_ADJUST (unsigned @var{nunroll}, class loop *@var{loop})
 This target hook returns a new value for the number of times @var{loop}
 should be unrolled. The parameter @var{nunroll} is the number of times
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 9e0830758ae..87a7f895174 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7923,6 +7923,8 @@ lists.
 
 @hook TARGET_GEN_CCMP_NEXT
 
+@hook TARGET_HAVE_CCMP
+
 @hook TARGET_LOOP_UNROLL_ADJUST
 
 @defmac POWI_MAX_MULTS
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 1baa39b98eb..04bad5e1425 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -11089,7 +11089,7 @@ expand_expr_real_gassign (gassign *g, rtx target, machine_mode tmode,
       ops.op1 = gimple_assign_rhs2 (g);
 
       /* Try to expand conditonal compare.  */
-      if (targetm.gen_ccmp_first)
+      if (targetm.have_ccmp ())
 	{
 	  gcc_checking_assert (targetm.gen_ccmp_next != NULL);
 	  r = expand_ccmp_expr (g, TYPE_MODE (ops.type));
diff --git a/gcc/target.def b/gcc/target.def
index 70070caebc7..1511038785d 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2783,6 +2783,15 @@ DEFHOOK
  rtx, (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev, rtx_code cmp_code, tree op0, tree op1, rtx_code bit_code),
  NULL)
 
+/* Return true if the target supports conditional compare.  */
+DEFHOOK
+(have_ccmp,
+ "This target hook returns true if the target supports conditional compare.\n\
+This target hook is required only when the target has conditional compare that\n\
+was not enabled by default, such as x86-64.",
+ bool, (void),
+ default_have_ccmp)
+
 /* Return a new value for loop unroll size.  */
 DEFHOOK
 (loop_unroll_adjust,
diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
index fb339bf75dd..4f53257e55c 100644
--- a/gcc/targhooks.cc
+++ b/gcc/targhooks.cc
@@ -1887,6 +1887,12 @@ default_have_conditional_execution (void)
   return HAVE_conditional_execution;
 }
 
+bool
+default_have_ccmp (void)
+{
+  return targetm.gen_ccmp_first != NULL;
+}
+
 /* By default we assume that c99 functions are present at the runtime,
    but sincos is not.  */
 bool
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 85f3817c176..f53913ebdfa 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -216,6 +216,7 @@ extern void default_addr_space_diagnose_usage (addr_space_t, location_t);
 extern rtx default_addr_space_convert (rtx, tree, tree);
 extern unsigned int default_case_values_threshold (void);
 extern bool default_have_conditional_execution (void);
+extern bool default_have_ccmp (void);
 
 extern bool default_libc_has_function (enum function_class, tree);
 extern bool default_libc_has_fast_function (int fcode);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Add targetm.have_ccmp hook [PR115370]
  2024-06-13  2:43 [PATCH] Add targetm.have_ccmp hook [PR115370] Hongyu Wang
@ 2024-06-13  9:03 ` Richard Sandiford
  2024-06-13 13:19   ` Hongyu Wang
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Sandiford @ 2024-06-13  9:03 UTC (permalink / raw)
  To: Hongyu Wang; +Cc: gcc-patches, hongtao.liu, ubizjak, richard.guenther

Hongyu Wang <hongyu.wang@intel.com> writes:
> Hi,
>
> In cfgexpand, there is an optimization for branch which tests
> targetm.gen_ccmp_first == NULL. However for target like x86-64, the
> hook was implemented but it does not indicate that ccmp was enabled.
> Add a new target hook TARGET_HAVE_CCMP and replace the middle-end
> check for the existance of gen_ccmp_first to avoid misoptimization.
>
> This fixes PR115370 that have suboptimal codegen, also I checked the
> znver2 binary for 526 and it will have same binary as the one before
> the CCMP support patch r15-1058, so suppose it could also fix PR115463.

It's unfortunate that we can't simply try expanding ccmp and seeing
whether it works, but I agree that that isn't possible for the
cfgexpand.cc change.

The expr.cc change shouldn't be needed, but I agree it's more consistent
to change both places in the same way.

> Bootstrapped/regtested on x86-64-pc-linux-gnu and aarch64-none-linux-gnu.
>
> Ok for trunk?
>
> gcc/ChangeLog:
>
> 	PR target/115370
> 	PR target/115463
> 	* cfgexpand.cc (expand_gimple_cond): Call targetm.have_ccmp
> 	instead of checking if targetm.gen_ccmp_first exists.
> 	* expr.cc (expand_expr_real_gassign): Likewise.
> 	* config/i386/i386.cc (ix86_have_ccmp): New target hook to
> 	check if APX_CCMP enabled.
> 	(TARGET_HAVE_CCMP): Define.
> 	* doc/tm.texi: Add TARGET_HAVE_CCMP.
> 	* doc/tm.texi.in: Regenerated.
> 	* target.def (TARGET_HAVE_CCMP): New target hook.
> 	* targhooks.cc (default_have_ccmp): New function.
> 	* targhooks.h (default_have_ccmp): New prototype.
> ---
>  gcc/cfgexpand.cc        | 2 +-
>  gcc/config/i386/i386.cc | 9 +++++++++
>  gcc/doc/tm.texi         | 6 ++++++
>  gcc/doc/tm.texi.in      | 2 ++
>  gcc/expr.cc             | 2 +-
>  gcc/target.def          | 9 +++++++++
>  gcc/targhooks.cc        | 6 ++++++
>  gcc/targhooks.h         | 1 +
>  8 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> index 8de5f2ba58b..dad3ae1b7c6 100644
> --- a/gcc/cfgexpand.cc
> +++ b/gcc/cfgexpand.cc
> @@ -2646,7 +2646,7 @@ expand_gimple_cond (basic_block bb, gcond *stmt)
>  	  /* If jumps are cheap and the target does not support conditional
>  	     compare, turn some more codes into jumpy sequences.  */
>  	  else if (BRANCH_COST (optimize_insn_for_speed_p (), false) < 4
> -		   && targetm.gen_ccmp_first == NULL)
> +		   && !targetm.have_ccmp ())
>  	    {
>  	      if ((code2 == BIT_AND_EXPR
>  		   && TYPE_PRECISION (TREE_TYPE (op0)) == 1
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 173db213d14..c72f64da983 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -26204,6 +26204,13 @@ ix86_memtag_add_tag (rtx base, poly_int64 offset, unsigned char tag_offset)
>    return plus_constant (Pmode, tagged_addr, offset);
>  }
>  
> +/* Implement TARGET_HAVE_CCMP.  */
> +static bool
> +ix86_have_ccmp ()
> +{
> +  return (bool) TARGET_APX_CCMP;
> +}
> +
>  /* Target-specific selftests.  */
>  
>  #if CHECKING_P
> @@ -27043,6 +27050,8 @@ ix86_libgcc_floating_mode_supported_p
>  #undef TARGET_GEN_CCMP_NEXT
>  #define TARGET_GEN_CCMP_NEXT ix86_gen_ccmp_next
>  
> +#undef TARGET_HAVE_CCMP
> +#define TARGET_HAVE_CCMP ix86_have_ccmp
>  
>  static bool
>  ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED)
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 8a7aa70d605..993816deeba 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -12354,6 +12354,12 @@ This function prepares to emit a conditional comparison within a sequence
>   @var{bit_code} is @code{AND} or @code{IOR}, which is the op on the compares.
>  @end deftypefn
>  
> +@deftypefn {Target Hook} bool TARGET_HAVE_CCMP (void)
> +This target hook returns true if the target supports conditional compare.
> +This target hook is required only when the target has conditional compare that
> +was not enabled by default, such as x86-64.

Maybe the second sentence could be something like:

-----
The hook is required only when the support is conditionally enabled,
such as in response to command-line flags.  The default implementation
returns true iff @code{TARGET_GEN_CCMP_FIRST} is defined.
-----

The reason for the suggestion is that "was enabled by default" sounds to
me like it's describing behaviour that can be altered by command-line flags,
rather than something that happens unconditionally.

OK with that change, thanks.

Richard

> +@end deftypefn
> +
>  @deftypefn {Target Hook} unsigned TARGET_LOOP_UNROLL_ADJUST (unsigned @var{nunroll}, class loop *@var{loop})
>  This target hook returns a new value for the number of times @var{loop}
>  should be unrolled. The parameter @var{nunroll} is the number of times
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 9e0830758ae..87a7f895174 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -7923,6 +7923,8 @@ lists.
>  
>  @hook TARGET_GEN_CCMP_NEXT
>  
> +@hook TARGET_HAVE_CCMP
> +
>  @hook TARGET_LOOP_UNROLL_ADJUST
>  
>  @defmac POWI_MAX_MULTS
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index 1baa39b98eb..04bad5e1425 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -11089,7 +11089,7 @@ expand_expr_real_gassign (gassign *g, rtx target, machine_mode tmode,
>        ops.op1 = gimple_assign_rhs2 (g);
>  
>        /* Try to expand conditonal compare.  */
> -      if (targetm.gen_ccmp_first)
> +      if (targetm.have_ccmp ())
>  	{
>  	  gcc_checking_assert (targetm.gen_ccmp_next != NULL);
>  	  r = expand_ccmp_expr (g, TYPE_MODE (ops.type));
> diff --git a/gcc/target.def b/gcc/target.def
> index 70070caebc7..1511038785d 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -2783,6 +2783,15 @@ DEFHOOK
>   rtx, (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev, rtx_code cmp_code, tree op0, tree op1, rtx_code bit_code),
>   NULL)
>  
> +/* Return true if the target supports conditional compare.  */
> +DEFHOOK
> +(have_ccmp,
> + "This target hook returns true if the target supports conditional compare.\n\
> +This target hook is required only when the target has conditional compare that\n\
> +was not enabled by default, such as x86-64.",
> + bool, (void),
> + default_have_ccmp)
> +
>  /* Return a new value for loop unroll size.  */
>  DEFHOOK
>  (loop_unroll_adjust,
> diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> index fb339bf75dd..4f53257e55c 100644
> --- a/gcc/targhooks.cc
> +++ b/gcc/targhooks.cc
> @@ -1887,6 +1887,12 @@ default_have_conditional_execution (void)
>    return HAVE_conditional_execution;
>  }
>  
> +bool
> +default_have_ccmp (void)
> +{
> +  return targetm.gen_ccmp_first != NULL;
> +}
> +
>  /* By default we assume that c99 functions are present at the runtime,
>     but sincos is not.  */
>  bool
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index 85f3817c176..f53913ebdfa 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -216,6 +216,7 @@ extern void default_addr_space_diagnose_usage (addr_space_t, location_t);
>  extern rtx default_addr_space_convert (rtx, tree, tree);
>  extern unsigned int default_case_values_threshold (void);
>  extern bool default_have_conditional_execution (void);
> +extern bool default_have_ccmp (void);
>  
>  extern bool default_libc_has_function (enum function_class, tree);
>  extern bool default_libc_has_fast_function (int fcode);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Add targetm.have_ccmp hook [PR115370]
  2024-06-13  9:03 ` Richard Sandiford
@ 2024-06-13 13:19   ` Hongyu Wang
  0 siblings, 0 replies; 3+ messages in thread
From: Hongyu Wang @ 2024-06-13 13:19 UTC (permalink / raw)
  To: Hongyu Wang, gcc-patches, hongtao.liu, ubizjak, richard.guenther,
	richard.sandiford

[-- Attachment #1: Type: text/plain, Size: 8163 bytes --]

Thanks, this it the patch I'm going to check-in.

Richard Sandiford <richard.sandiford@arm.com> 于2024年6月13日周四 17:04写道:
>
> Hongyu Wang <hongyu.wang@intel.com> writes:
> > Hi,
> >
> > In cfgexpand, there is an optimization for branch which tests
> > targetm.gen_ccmp_first == NULL. However for target like x86-64, the
> > hook was implemented but it does not indicate that ccmp was enabled.
> > Add a new target hook TARGET_HAVE_CCMP and replace the middle-end
> > check for the existance of gen_ccmp_first to avoid misoptimization.
> >
> > This fixes PR115370 that have suboptimal codegen, also I checked the
> > znver2 binary for 526 and it will have same binary as the one before
> > the CCMP support patch r15-1058, so suppose it could also fix PR115463.
>
> It's unfortunate that we can't simply try expanding ccmp and seeing
> whether it works, but I agree that that isn't possible for the
> cfgexpand.cc change.
>
> The expr.cc change shouldn't be needed, but I agree it's more consistent
> to change both places in the same way.
>
> > Bootstrapped/regtested on x86-64-pc-linux-gnu and aarch64-none-linux-gnu.
> >
> > Ok for trunk?
> >
> > gcc/ChangeLog:
> >
> >       PR target/115370
> >       PR target/115463
> >       * cfgexpand.cc (expand_gimple_cond): Call targetm.have_ccmp
> >       instead of checking if targetm.gen_ccmp_first exists.
> >       * expr.cc (expand_expr_real_gassign): Likewise.
> >       * config/i386/i386.cc (ix86_have_ccmp): New target hook to
> >       check if APX_CCMP enabled.
> >       (TARGET_HAVE_CCMP): Define.
> >       * doc/tm.texi: Add TARGET_HAVE_CCMP.
> >       * doc/tm.texi.in: Regenerated.
> >       * target.def (TARGET_HAVE_CCMP): New target hook.
> >       * targhooks.cc (default_have_ccmp): New function.
> >       * targhooks.h (default_have_ccmp): New prototype.
> > ---
> >  gcc/cfgexpand.cc        | 2 +-
> >  gcc/config/i386/i386.cc | 9 +++++++++
> >  gcc/doc/tm.texi         | 6 ++++++
> >  gcc/doc/tm.texi.in      | 2 ++
> >  gcc/expr.cc             | 2 +-
> >  gcc/target.def          | 9 +++++++++
> >  gcc/targhooks.cc        | 6 ++++++
> >  gcc/targhooks.h         | 1 +
> >  8 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> > index 8de5f2ba58b..dad3ae1b7c6 100644
> > --- a/gcc/cfgexpand.cc
> > +++ b/gcc/cfgexpand.cc
> > @@ -2646,7 +2646,7 @@ expand_gimple_cond (basic_block bb, gcond *stmt)
> >         /* If jumps are cheap and the target does not support conditional
> >            compare, turn some more codes into jumpy sequences.  */
> >         else if (BRANCH_COST (optimize_insn_for_speed_p (), false) < 4
> > -                && targetm.gen_ccmp_first == NULL)
> > +                && !targetm.have_ccmp ())
> >           {
> >             if ((code2 == BIT_AND_EXPR
> >                  && TYPE_PRECISION (TREE_TYPE (op0)) == 1
> > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> > index 173db213d14..c72f64da983 100644
> > --- a/gcc/config/i386/i386.cc
> > +++ b/gcc/config/i386/i386.cc
> > @@ -26204,6 +26204,13 @@ ix86_memtag_add_tag (rtx base, poly_int64 offset, unsigned char tag_offset)
> >    return plus_constant (Pmode, tagged_addr, offset);
> >  }
> >
> > +/* Implement TARGET_HAVE_CCMP.  */
> > +static bool
> > +ix86_have_ccmp ()
> > +{
> > +  return (bool) TARGET_APX_CCMP;
> > +}
> > +
> >  /* Target-specific selftests.  */
> >
> >  #if CHECKING_P
> > @@ -27043,6 +27050,8 @@ ix86_libgcc_floating_mode_supported_p
> >  #undef TARGET_GEN_CCMP_NEXT
> >  #define TARGET_GEN_CCMP_NEXT ix86_gen_ccmp_next
> >
> > +#undef TARGET_HAVE_CCMP
> > +#define TARGET_HAVE_CCMP ix86_have_ccmp
> >
> >  static bool
> >  ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED)
> > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> > index 8a7aa70d605..993816deeba 100644
> > --- a/gcc/doc/tm.texi
> > +++ b/gcc/doc/tm.texi
> > @@ -12354,6 +12354,12 @@ This function prepares to emit a conditional comparison within a sequence
> >   @var{bit_code} is @code{AND} or @code{IOR}, which is the op on the compares.
> >  @end deftypefn
> >
> > +@deftypefn {Target Hook} bool TARGET_HAVE_CCMP (void)
> > +This target hook returns true if the target supports conditional compare.
> > +This target hook is required only when the target has conditional compare that
> > +was not enabled by default, such as x86-64.
>
> Maybe the second sentence could be something like:
>
> -----
> The hook is required only when the support is conditionally enabled,
> such as in response to command-line flags.  The default implementation
> returns true iff @code{TARGET_GEN_CCMP_FIRST} is defined.
> -----
>
> The reason for the suggestion is that "was enabled by default" sounds to
> me like it's describing behaviour that can be altered by command-line flags,
> rather than something that happens unconditionally.
>
> OK with that change, thanks.
>
> Richard
>
> > +@end deftypefn
> > +
> >  @deftypefn {Target Hook} unsigned TARGET_LOOP_UNROLL_ADJUST (unsigned @var{nunroll}, class loop *@var{loop})
> >  This target hook returns a new value for the number of times @var{loop}
> >  should be unrolled. The parameter @var{nunroll} is the number of times
> > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> > index 9e0830758ae..87a7f895174 100644
> > --- a/gcc/doc/tm.texi.in
> > +++ b/gcc/doc/tm.texi.in
> > @@ -7923,6 +7923,8 @@ lists.
> >
> >  @hook TARGET_GEN_CCMP_NEXT
> >
> > +@hook TARGET_HAVE_CCMP
> > +
> >  @hook TARGET_LOOP_UNROLL_ADJUST
> >
> >  @defmac POWI_MAX_MULTS
> > diff --git a/gcc/expr.cc b/gcc/expr.cc
> > index 1baa39b98eb..04bad5e1425 100644
> > --- a/gcc/expr.cc
> > +++ b/gcc/expr.cc
> > @@ -11089,7 +11089,7 @@ expand_expr_real_gassign (gassign *g, rtx target, machine_mode tmode,
> >        ops.op1 = gimple_assign_rhs2 (g);
> >
> >        /* Try to expand conditonal compare.  */
> > -      if (targetm.gen_ccmp_first)
> > +      if (targetm.have_ccmp ())
> >       {
> >         gcc_checking_assert (targetm.gen_ccmp_next != NULL);
> >         r = expand_ccmp_expr (g, TYPE_MODE (ops.type));
> > diff --git a/gcc/target.def b/gcc/target.def
> > index 70070caebc7..1511038785d 100644
> > --- a/gcc/target.def
> > +++ b/gcc/target.def
> > @@ -2783,6 +2783,15 @@ DEFHOOK
> >   rtx, (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev, rtx_code cmp_code, tree op0, tree op1, rtx_code bit_code),
> >   NULL)
> >
> > +/* Return true if the target supports conditional compare.  */
> > +DEFHOOK
> > +(have_ccmp,
> > + "This target hook returns true if the target supports conditional compare.\n\
> > +This target hook is required only when the target has conditional compare that\n\
> > +was not enabled by default, such as x86-64.",
> > + bool, (void),
> > + default_have_ccmp)
> > +
> >  /* Return a new value for loop unroll size.  */
> >  DEFHOOK
> >  (loop_unroll_adjust,
> > diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> > index fb339bf75dd..4f53257e55c 100644
> > --- a/gcc/targhooks.cc
> > +++ b/gcc/targhooks.cc
> > @@ -1887,6 +1887,12 @@ default_have_conditional_execution (void)
> >    return HAVE_conditional_execution;
> >  }
> >
> > +bool
> > +default_have_ccmp (void)
> > +{
> > +  return targetm.gen_ccmp_first != NULL;
> > +}
> > +
> >  /* By default we assume that c99 functions are present at the runtime,
> >     but sincos is not.  */
> >  bool
> > diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> > index 85f3817c176..f53913ebdfa 100644
> > --- a/gcc/targhooks.h
> > +++ b/gcc/targhooks.h
> > @@ -216,6 +216,7 @@ extern void default_addr_space_diagnose_usage (addr_space_t, location_t);
> >  extern rtx default_addr_space_convert (rtx, tree, tree);
> >  extern unsigned int default_case_values_threshold (void);
> >  extern bool default_have_conditional_execution (void);
> > +extern bool default_have_ccmp (void);
> >
> >  extern bool default_libc_has_function (enum function_class, tree);
> >  extern bool default_libc_has_fast_function (int fcode);

[-- Attachment #2: 0001-Add-targetm.have_ccmp-hook.patch --]
[-- Type: text/x-patch, Size: 6261 bytes --]

From c9883121f14c34f52769652fd5c6203758b5f780 Mon Sep 17 00:00:00 2001
From: Hongyu Wang <hongyu.wang@intel.com>
Date: Thu, 13 Jun 2024 00:18:32 +0800
Subject: [PATCH] Add targetm.have_ccmp hook

In cfgexpand, there is an optimization for branch which tests
targetm.gen_ccmp_first == NULL. However for target like x86-64, the
hook was implemented but it does not indicate that ccmp was enabled.
Add a new target hook TARGET_HAVE_CCMP and replace the middle-end
check for the existance of gen_ccmp_first to avoid misoptimization.

gcc/ChangeLog:

	PR target/115370
	PR target/115463
	* cfgexpand.cc (expand_gimple_cond): Call targetm.have_ccmp
	instead of checking if targetm.gen_ccmp_first exists.
	* expr.cc (expand_expr_real_gassign): Likewise.
	* config/i386/i386.cc (ix86_have_ccmp): New target hook to
	check if APX_CCMP enabled.
	(TARGET_HAVE_CCMP): Define.
	* doc/tm.texi: Add TARGET_HAVE_CCMP.
	* doc/tm.texi.in: Regenerated.
	* target.def (TARGET_HAVE_CCMP): New target hook.
	* targhooks.cc (default_have_ccmp): New function.
	* targhooks.h (default_have_ccmp): New prototype.
---
 gcc/cfgexpand.cc        |  2 +-
 gcc/config/i386/i386.cc |  9 +++++++++
 gcc/doc/tm.texi         |  7 +++++++
 gcc/doc/tm.texi.in      |  2 ++
 gcc/expr.cc             |  2 +-
 gcc/target.def          | 10 ++++++++++
 gcc/targhooks.cc        |  6 ++++++
 gcc/targhooks.h         |  1 +
 8 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 8de5f2ba58b..dad3ae1b7c6 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -2646,7 +2646,7 @@ expand_gimple_cond (basic_block bb, gcond *stmt)
 	  /* If jumps are cheap and the target does not support conditional
 	     compare, turn some more codes into jumpy sequences.  */
 	  else if (BRANCH_COST (optimize_insn_for_speed_p (), false) < 4
-		   && targetm.gen_ccmp_first == NULL)
+		   && !targetm.have_ccmp ())
 	    {
 	      if ((code2 == BIT_AND_EXPR
 		   && TYPE_PRECISION (TREE_TYPE (op0)) == 1
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index 173db213d14..c72f64da983 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -26204,6 +26204,13 @@ ix86_memtag_add_tag (rtx base, poly_int64 offset, unsigned char tag_offset)
   return plus_constant (Pmode, tagged_addr, offset);
 }
 
+/* Implement TARGET_HAVE_CCMP.  */
+static bool
+ix86_have_ccmp ()
+{
+  return (bool) TARGET_APX_CCMP;
+}
+
 /* Target-specific selftests.  */
 
 #if CHECKING_P
@@ -27043,6 +27050,8 @@ ix86_libgcc_floating_mode_supported_p
 #undef TARGET_GEN_CCMP_NEXT
 #define TARGET_GEN_CCMP_NEXT ix86_gen_ccmp_next
 
+#undef TARGET_HAVE_CCMP
+#define TARGET_HAVE_CCMP ix86_have_ccmp
 
 static bool
 ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED)
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 8a7aa70d605..be5543b72f8 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12354,6 +12354,13 @@ This function prepares to emit a conditional comparison within a sequence
  @var{bit_code} is @code{AND} or @code{IOR}, which is the op on the compares.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_HAVE_CCMP (void)
+This target hook returns true if the target supports conditional compare.
+This target hook is required only when the ccmp support is conditionally
+enabled, such as in response to command-line flags. The default implementation
+returns true iff @code{TARGET_GEN_CCMP_FIRST} is defined.
+@end deftypefn
+
 @deftypefn {Target Hook} unsigned TARGET_LOOP_UNROLL_ADJUST (unsigned @var{nunroll}, class loop *@var{loop})
 This target hook returns a new value for the number of times @var{loop}
 should be unrolled. The parameter @var{nunroll} is the number of times
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 9e0830758ae..87a7f895174 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7923,6 +7923,8 @@ lists.
 
 @hook TARGET_GEN_CCMP_NEXT
 
+@hook TARGET_HAVE_CCMP
+
 @hook TARGET_LOOP_UNROLL_ADJUST
 
 @defmac POWI_MAX_MULTS
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 1baa39b98eb..04bad5e1425 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -11089,7 +11089,7 @@ expand_expr_real_gassign (gassign *g, rtx target, machine_mode tmode,
       ops.op1 = gimple_assign_rhs2 (g);
 
       /* Try to expand conditonal compare.  */
-      if (targetm.gen_ccmp_first)
+      if (targetm.have_ccmp ())
 	{
 	  gcc_checking_assert (targetm.gen_ccmp_next != NULL);
 	  r = expand_ccmp_expr (g, TYPE_MODE (ops.type));
diff --git a/gcc/target.def b/gcc/target.def
index 70070caebc7..e5a9b52676e 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2783,6 +2783,16 @@ DEFHOOK
  rtx, (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev, rtx_code cmp_code, tree op0, tree op1, rtx_code bit_code),
  NULL)
 
+/* Return true if the target supports conditional compare.  */
+DEFHOOK
+(have_ccmp,
+ "This target hook returns true if the target supports conditional compare.\n\
+This target hook is required only when the ccmp support is conditionally\n\
+enabled, such as in response to command-line flags. The default implementation\n\
+returns true iff @code{TARGET_GEN_CCMP_FIRST} is defined.",
+ bool, (void),
+ default_have_ccmp)
+
 /* Return a new value for loop unroll size.  */
 DEFHOOK
 (loop_unroll_adjust,
diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
index fb339bf75dd..4f53257e55c 100644
--- a/gcc/targhooks.cc
+++ b/gcc/targhooks.cc
@@ -1887,6 +1887,12 @@ default_have_conditional_execution (void)
   return HAVE_conditional_execution;
 }
 
+bool
+default_have_ccmp (void)
+{
+  return targetm.gen_ccmp_first != NULL;
+}
+
 /* By default we assume that c99 functions are present at the runtime,
    but sincos is not.  */
 bool
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 85f3817c176..f53913ebdfa 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -216,6 +216,7 @@ extern void default_addr_space_diagnose_usage (addr_space_t, location_t);
 extern rtx default_addr_space_convert (rtx, tree, tree);
 extern unsigned int default_case_values_threshold (void);
 extern bool default_have_conditional_execution (void);
+extern bool default_have_ccmp (void);
 
 extern bool default_libc_has_function (enum function_class, tree);
 extern bool default_libc_has_fast_function (int fcode);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-06-13 13:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-13  2:43 [PATCH] Add targetm.have_ccmp hook [PR115370] Hongyu Wang
2024-06-13  9:03 ` Richard Sandiford
2024-06-13 13:19   ` Hongyu Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).