public inbox for
 help / color / mirror / Atom feed
From: Segher Boessenkool <>
To: Roger Sayle <>
Subject: Re: [PATCH] Be careful with MODE_CC in simplify_const_relational_operation.
Date: Thu, 7 Jul 2022 17:38:33 -0500	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <003601d89245$a86f8830$f94e9890$>


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, 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?


  reply	other threads:[~2022-07-07 22:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-07 21:08 Roger Sayle
2022-07-07 22:38 ` Segher Boessenkool [this message]
2022-07-26 12:13   ` [PATCH] Add new target hook: simplify_modecc_const Roger Sayle
2022-07-26 17:44     ` Segher Boessenkool
2022-07-26 21:04       ` Roger Sayle
2022-07-26 22:11         ` Segher Boessenkool
2022-07-27  7:51           ` Roger Sayle
2022-07-27 18:37             ` Segher Boessenkool
2022-07-28 12:39         ` Richard Sandiford
2022-10-10 15:50           ` H.J. Lu
2022-10-14 20:31             ` Jeff Law
2022-10-14 21:05               ` H.J. Lu
2022-10-14 20:26       ` Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).