Re: [PATCH] Add new target hook: simplify_modecc_const.

[Jeff Law <jeffreyalaw@gmail.com>]

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


jeff