[PATCH] Add new target hook: simplify_modecc_const.

["Roger Sayle" <roger@nextmovesoftware.com>]

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


This patch is a major revision of the patch I originally proposed here:
https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598040.html

The primary motivation of this patch is to avoid incorrect optimization
of MODE_CC comparisons in simplify_const_relational_operation when/if a
backend represents the (known) contents of a MODE_CC register using a
CONST_INT.  In such cases, the RTL optimizers don't know the semantics
of this integer value, so shouldn't change anything (i.e. should return
NULL_RTX from simplify_const_relational_operation).

The secondary motivation is that by introducing a new target hook, called
simplify_modecc_const, the backend can (optionally) encode and interpret
a target dependent encoding of MODE_CC registers.

The worked example provided with this patch is to allow the i386 backend
to explicitly model the carry flag (MODE_CCC) using 1 to indicate that
the carry flag is set, and 0 to indicate the carry flag is clear.  This
allows the instructions stc (set carry flag), clc (clear carry flag) and
cmc (complement carry flag) to be represented in RTL.

However an even better example would be the rs6000 backend, where this
patch/target hook would allow improved modelling of the condition register
CR.  The powerpc's comparison instructions set fields/bits in the CR
register [where bit 0 indicates less than, bit 1 greater than, bit 2
equal to and bit3 overflow] analogous to x86's flags register [containing
bits for carry, zero, overflow, parity etc.].  These fields can be
manipulated directly using crset (aka creqv) and crclr (aka crxor)
instructions and even transferred from general purpose registers using
mtcr.  However, without a patch like this, it's impossible to safely
model/represent these instructions in rs6000.md.

This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target_board=unix{-m32},
and both with and without a patch to add stc, clc and cmc support to
the x86 backend.  I'll resubmit the x86 target pieces again with that
follow-up backend patch, so for now I'm only looking for approval
of the middle-end infrastructure pieces.  The x86 hunks below are
provided as context/documentation for how this hook could/should be
used (but I wouldn't object to pre-approval of those bits by Uros).
Ok for mainline?


2022-07-26  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        * target.def (simplify_modecc_const): New target hook.
        * doc/tm.texi (TARGET_SIMPLIFY_MODECC_CONST): Document here.
        * doc/tm.texi.in (TARGET_SIMPLIFY_MODECC_CONST): Locate @hook here.
        * hooks.cc (hook_rtx_mode_int_rtx_null): Define default hook here.
        * hooks.h (hook_rtx_mode_int_rtx_null): Prototype here.
        * simplify-rtx.c (simplify_const_relational_operation): Avoid
        mis-optimizing MODE_CC comparisons by calling new target hook.

        * config/i386.cc (ix86_simplify_modecc_const): Implement new target
        hook, supporting interpreting MODE_CCC values as the x86 carry flag.
        (TARGET_SIMPLIFY_MODECC_CONST): Define as
ix86_simplify_modecc_const.


Thanks in advance,
Roger
--

> -----Original Message-----
> From: Segher Boessenkool <segher@kernel.crashing.org>
> Sent: 07 July 2022 23:39
> To: Roger Sayle <roger@nextmovesoftware.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Be careful with MODE_CC in
> simplify_const_relational_operation.
> 
> Hi!
> 
> On Thu, Jul 07, 2022 at 10:08:04PM +0100, Roger Sayle wrote:
> > I think it's fair to describe RTL's representation of condition flags
> > using MODE_CC as a little counter-intuitive.
> 
> "A little challenging", and you should see that as a good thing, as a
puzzle to
> crack :-)
> 
> > For example, the i386
> > backend represents the carry flag (in adc instructions) using RTL of
> > the form "(ltu:SI (reg:CCC) (const_int 0))", where great care needs to
> > be taken not to treat this like a normal RTX expression, after all LTU
> > (less-than-unsigned) against const0_rtx would normally always be
> > false.
> 
> A comparison of a MODE_CC thing against 0 means the result of a
> *previous* comparison (or other cc setter) is looked at.  Usually it
simply looks
> at some condition bits in a flags register.  It does not do any actual
comparison:
> that has been done before (if at all even).
> 
> > Hence, MODE_CC comparisons need to be treated with caution, and
> > simplify_const_relational_operation returns early (to avoid
> > problems) when GET_MODE_CLASS (GET_MODE (op0)) == MODE_CC.
> 
> Not just to avoid problems: there simply isn't enough information to do a
> correct job.
> 
> > However, consider the (currently) hypothetical situation, where the
> > RTL optimizers determine that a previous instruction unconditionally
> > sets or clears the carry flag, and this gets propagated by combine
> > into the above expression, we'd end up with something that looks like
> > (ltu:SI (const_int 1) (const_int 0)), which doesn't mean what it says.
> > Fortunately, simplify_const_relational_operation is passed the
> > original mode of the comparison (cmp_mode, the original mode of op0)
> > which can be checked for MODE_CC, even when op0 is now VOIDmode
> > (const_int) after the substitution.  Defending against this is clearly
> > the right thing to do.
> >
> > More controversially, rather than just abort
> > simplification/optimization in this case, we can use the comparison
> > operator to infer/select the semantics of the CC_MODE flag.
> > Hopefully, whenever a backend uses LTU, it represents the (set) carry
> > flag (and behaves like i386.md), in which case the result of the
simplified
> expression is the first operand.
> > [If there's no standardization of semantics across backends, then we
> > should always just return 0; but then miss potential optimizations].
> 
> On PowerPC, ltu means the result of an unsigned comparison (we have
> instructions for that, cmpl[wd][i] mainly) was "smaller than".  It does
not mean
> anything is unsigned smaller than zero.  It also has nothing to do with
carries,
> which are done via a different register (the XER).
> 
> > +  /* Handle MODE_CC comparisons that have been simplified to
> > +     constants.  */
> > +  if (GET_MODE_CLASS (mode) == MODE_CC
> > +      && op1 == const0_rtx
> > +      && CONST_INT_P (op0))
> > +    {
> > +      /* LTU represents the carry flag.  */
> > +      if (code == LTU)
> > +	return op0 == const0_rtx ? const0_rtx : const_true_rtx;
> > +      return 0;
> > +    }
> > +
> >    /* We can't simplify MODE_CC values since we don't know what the
> >       actual comparison is.  */
> 
> ^^^
> This comment is 100% true.  We cannot simplify any MODE_CC comparison
> without having more context.  The combiner does have that context when it
> tries to combine the CC setter with the CC consumer, for example.
> 
> Do you have some piece of motivating example code?
> 
> 
> Segher

[-- Attachment #2: patchcf3.txt --]
[-- Type: text/plain, Size: 5959 bytes --]

diff --git a/gcc/target.def b/gcc/target.def
index 2a7fa68..cd81b4e 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -7107,6 +7107,20 @@ counters are incremented using atomic operations.  Targets not supporting\n\
 type.",
  HOST_WIDE_INT, (void), default_gcov_type_size)
 
+DEFHOOK
+(simplify_modecc_const,
+ "On targets that represent the result of a comparison using a\n\
+@code{MODE_CC} flags register, allow the backend to represent a known\n\
+result using a target-dependent integer constant.  This helper function\n\
+of @code{simplify_const_relational_operation} attempts to simplify a\n\
+comparison operator @var{code} that is defined by a @code{COMPARE} in mode\n\
+@var{mode}, which is known to be of class @code{MODE_CC}, and is encoded\n\
+as @var{val}, which must be a @code{CONST_INT}.  The result must be\n\
+@code{const_true_rtx}, @code{const0_rtx} or @code{NULL_RTX} if no\n\
+simplification is possible.",
+ rtx, (machine_mode mode, int code, rtx val),
+ hook_rtx_mode_int_rtx_null)
+
 /* This value represents whether the shadow call stack is implemented on
    the target platform.  */
 DEFHOOKPOD
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index b0ea398..5a1231d 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6631,6 +6631,18 @@ post-reload comparison elimination pass, or the delay slot filler pass,
 then this value should be set appropriately.
 @end deftypevr
 
+@deftypefn {Target Hook} rtx TARGET_SIMPLIFY_MODECC_CONST (machine_mode @var{mode}, int @var{code}, rtx @var{val})
+On targets that represent the result of a comparison using a
+@code{MODE_CC} flags register, allow the backend to represent a known
+result using a target-dependent integer constant.  This helper function
+of @code{simplify_const_relational_operation} attempts to simplify a
+comparison operator @var{code} that is defined by a @code{COMPARE} in mode
+@var{mode}, which is known to be of class @code{MODE_CC}, and is encoded
+as @var{val}, which must be a @code{CONST_INT}.  The result must be
+@code{const_true_rtx}, @code{const0_rtx} or @code{NULL_RTX} if no
+simplification is possible.
+@end deftypefn
+
 @node Costs
 @section Describing Relative Costs of Operations
 @cindex costs of instructions
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index f869ddd..8cecde7 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4397,6 +4397,8 @@ like:
 
 @hook TARGET_FLAGS_REGNUM
 
+@hook TARGET_SIMPLIFY_MODECC_CONST
+
 @node Costs
 @section Describing Relative Costs of Operations
 @cindex costs of instructions
diff --git a/gcc/hooks.cc b/gcc/hooks.cc
index b29233f..90be303 100644
--- a/gcc/hooks.cc
+++ b/gcc/hooks.cc
@@ -401,6 +401,14 @@ hook_rtx_tree_int_null (tree, int)
   return NULL;
 }
 
+/* Generic hook that takes a machine mode, an int and an rtx and
+   returns NULL_RTX.  */
+rtx
+hook_rtx_mode_int_rtx_null (machine_mode, int, rtx)
+{
+  return NULL;
+}
+
 /* Generic hook that takes a machine mode and returns an unsigned int 0.  */
 unsigned int
 hook_uint_mode_0 (machine_mode)
diff --git a/gcc/hooks.h b/gcc/hooks.h
index 1056e1e..e978817 100644
--- a/gcc/hooks.h
+++ b/gcc/hooks.h
@@ -123,6 +123,7 @@ extern bool default_can_output_mi_thunk_no_vcall (const_tree, HOST_WIDE_INT,
 extern rtx hook_rtx_rtx_identity (rtx);
 extern rtx hook_rtx_rtx_null (rtx);
 extern rtx hook_rtx_tree_int_null (tree, int);
+extern rtx hook_rtx_mode_int_rtx_null (machine_mode, int, rtx);
 
 extern char *hook_charptr_void_null (void);
 extern const char *hook_constcharptr_void_null (void);
diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index fa20665..699e08a 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -6012,6 +6012,13 @@ simplify_const_relational_operation (enum rtx_code code,
 	      || (GET_MODE (op0) == VOIDmode
 		  && GET_MODE (op1) == VOIDmode));
 
+  /* Handle MODE_CC comparisons that have been simplified to
+     constants.  */
+  if (GET_MODE_CLASS (mode) == MODE_CC
+      && op1 == const0_rtx
+      && CONST_INT_P (op0))
+    return targetm.simplify_modecc_const (mode, (int)code, op0);
+
   /* If op0 is a compare, extract the comparison arguments from it.  */
   if (GET_CODE (op0) == COMPARE && op1 == const0_rtx)
     {
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index e03f86d..a73c265 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -15995,6 +15995,31 @@ ix86_cc_modes_compatible (machine_mode m1, machine_mode m2)
     }
 }
 
+/* Simplify the results of comparisons with constant operands.
+   MODE is a MODE_CC, CODE is a comparison operator, and VAL is
+   a target (and mode) dependent CONST_INT.  */
+
+rtx
+ix86_simplify_modecc_const (machine_mode mode, int code, rtx val)
+{
+  /* For CCCmode, const0_rtx represents the carry flag is clear,
+     otherwise the carry flag is set.  */
+  switch ((enum rtx_code)code)
+    {
+    case LTU:
+      if (mode == E_CCCmode)
+	return val == const0_rtx ? const0_rtx : const_true_rtx;
+      break;
+    case GEU:
+      if (mode == E_CCCmode)
+	return val == const0_rtx ? const_true_rtx : const0_rtx;
+      break;
+    default:
+      break;
+    }
+  return NULL_RTX;
+}
+
 /* Return strategy to use for floating-point.  We assume that fcomi is always
    preferrable where available, since that is also true when looking at size
    (2 bytes, vs. 3 for fnstsw+sahf and at least 5 for fnstsw+test).  */
@@ -24978,6 +25003,9 @@ static bool ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED)
 #undef TARGET_GEN_MEMSET_SCRATCH_RTX
 #define TARGET_GEN_MEMSET_SCRATCH_RTX ix86_gen_scratch_sse_rtx
 
+#undef TARGET_SIMPLIFY_MODECC_CONST
+#define TARGET_SIMPLIFY_MODECC_CONST ix86_simplify_modecc_const
+
 #if CHECKING_P
 #undef TARGET_RUN_TARGET_SELFTESTS
 #define TARGET_RUN_TARGET_SELFTESTS selftest::ix86_run_selftests