public inbox for
 help / color / mirror / Atom feed
From: Jeff Law <>
Subject: Re: [PATCH] Add new target hook: simplify_modecc_const.
Date: Fri, 14 Oct 2022 14:26:25 -0600	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 7/26/22 11:44, Segher Boessenkool wrote:
> Hi!
> On Tue, Jul 26, 2022 at 01:13:02PM +0100, Roger Sayle wrote:
>> This patch is a major revision of the patch I originally proposed here:
>> 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).
> This is invalid RTL.  What would  (set (reg:CC) (const_int 0))  mean,
> for example?  If this was valid it would make most existing code using
> CC modes do essentially random things :-(

I'm not sure why you're claiming (set (reg:CC) (const_int 0)) is 
invalid.  I'm not aware of anything that would make it invalid -- but 
generic code doesn't really know how to interpret what it means.  While 
I have concerns in that space, they're pretty obscure and likely don't 
occur in practice due to the use of CC modes.

Not the interpretation of the underlying bits in the condition code is 
already defined as machine specific:

@findex CCmode

@item CCmode
``Condition Code'' mode represents the value of a condition code, which
is a machine-specific set of bits used to represent the result of a
comparison operation.  Other machine-specific modes may also be used for
the condition code.  (@pxref{Condition Code}).

Roger's patch does introduce special meaning to a relational operators 
in MODE_CC with two constants and I think that's really what you're 
concerned with and I would share a similar concern. Though perhaps not 
as severe as yours given how special MODE_CC has to be in many contexts.

I suspect we could probably all agree that rejecting a MODE_CC 
relational in simplify_const_relational_operation which would have been 
the minimal change to address the bug Roger is trying to fix.   I 
wouldn't be surprised if he started with that, then realized "hey, if we 
could ask the backend what 0 or 1 means for CC, then we could actually 
optimize this away completely and here we are...

> Comparing two integer constants is invalid RTL *in all contexts*.  The
> items compared do not have a mode!  From rtl.texi:
>    A @code{compare} specifying two @code{VOIDmode} constants is not valid
>    since there is no way to know in what mode the comparison is to be
>    performed; the comparison must either be folded during the compilation
>    or the first operand must be loaded into a register while its mode is
>    still known.

And I think the counter argument is that MODE_CC has enough special 
properties that it could be an exception to this rule. I'm not sure I'm 
ready to say, yes we should make this change, but I'm also not ready to 
summarily reject the change.


      parent reply	other threads:[~2022-10-14 20:26 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-07 21:08 [PATCH] Be careful with MODE_CC in simplify_const_relational_operation Roger Sayle
2022-07-07 22:38 ` Segher Boessenkool
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 [this message]

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).