Re: [PATCH] combine: Fix simplify_comparison AND handling for WORD_REGISTER_OPERATIONS targets [PR109040]

[Jeff Law <jeffreyalaw@gmail.com>]

On 4/6/23 05:37, Jakub Jelinek wrote:
> On Thu, Apr 06, 2023 at 12:51:20PM +0200, Eric Botcazou wrote:
>>> If we want to fix it in the combiner, I think the fix would be following.
>>> The optimization is about
>>> (and:SI (subreg:SI (reg:HI xxx) 0) (const_int 0x84c))
>>> and IMHO we can only optimize it into
>>> (subreg:SI (and:HI (reg:HI xxx) (const_int 0x84c)) 0)
>>> if we know that the upper bits of the REG are zeros.
>>
>> The reasoning is that, for WORD_REGISTER_OPERATIONS, the subword AND operation
>> is done on the full word register, in other words that it's in effect:
>>
>> (subreg:SI (and:SI (reg:SI xxx) (const_int 0x84c)) 0)
>>
>> that is equivalent to the initial RTL so correct for WORD_REGISTER_OPERATIONS.
> 
> If the
> (and:SI (subreg:SI (reg:HI xxx) 0) (const_int 0x84c))
> to
> (subreg:SI (and:HI (reg:HI xxx) (const_int 0x84c)) 0)
I think it is. In both cases the AND wipes the upper 16 bits.


> not really sure what for WORD_REGISTER_OPERATIONS
> means AND with a constant which has the most significant bit set for the
> upper bits.
That's a very good question.  I'm not sure either.  Obviously in the 
non-constant case all the bits up to word_mode get used.  The same thing 
is going to happen in the constant case.

THe fact that constants are sign extended from the mode bit is a gcc-ism 
though and not necessarily indicative of what hardware is going to to.



>>
>> What happens if you disable the step I mentioned (patchlet attached)?
> 
> That patch doesn't change anything at all on the testcase, it is still
> miscompiled.
That may be an artifact of later code in combine coming along and 
mucking things up in a manner similar.  That what I saw after twiddling 
simplify_binary_operation_1.  See simplify_and_const_int_1 and its calls 
to nonzero_bits

jeff