Re: [PATCH] Be careful with MODE_CC in simplify_const_relational_operation.

[Segher Boessenkool <segher@kernel.crashing.org>]

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