Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

[Jeff Law <jeffreyalaw@gmail.com>]

On 5/26/2021 11:29 AM, Andrew MacLeod via Gcc-patches wrote:
> On 5/26/21 7:07 AM, Andrew Pinski wrote:
>> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
>>> <bernd.edlinger@hotmail.de> wrote:
>>>> On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
>>>>> On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
>>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>>> From: Andrew Pinski <apinski@marvell.com>
>>>>>>
>>>>>> Instead of some of the more manual optimizations inside phi-opt,
>>>>>> it would be good idea to do a lot of the heavy lifting inside match
>>>>>> and simplify instead. In the process, this moves the three simple
>>>>>> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
>>>>>>
>>>>>> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
>>>>>>
>>>>>> Differences from V1:
>>>>>> * Use bit_xor 1 instead of bit_not to fix the problem with 
>>>>>> boolean types
>>>>>> which are not 1 bit precision.
>>>>> OK.
>>>>>
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>> Hmm, sorry, no luck.
>>>>
>>>> I think this caused:
>>> If anything it is a bad interaction with changes between r12-1046 and
>>> r12-1053; I am suspecting a bug in those changes rather than my
>>> changes causing the bug.  Debugging it right now.
>> (gdb) p debug_tree(name)
>>   <ssa_name 0x7ffff6a5cd38
>>      type <boolean_type 0x7ffff6b45b28 _Bool public unsigned QI
>>          size <integer_cst 0x7ffff6b2bdc8 constant 8>
>>          unit-size <integer_cst 0x7ffff6b2bde0 constant 1>
>>          align:8 warn_if_not_align:0 symtab:0 alias-set -1
>> canonical-type 0x7ffff6b45b28 precision:1 min <integer_cst
>> 0x7ffff6b4a030 0> max <integer_cst 0x7ffff6b4a060 1>>
>>
>>      def_stmt _19 = ~_8;
>>      version:19>
>>
>> So what is happening is evrp converted:
>> ct_12 = ct_5 + -1;
>> Into
>> ct_12 = ct_5 == 1 ? 0 : 1;
>> (this was done before my patch)
>> And then it gets simplified to:
>>    _8 = ct_5 == 1;
>>    _19 = ~_8;
>>    ct_12 = (int) _19;
>> (after my match.pd patch)
Yup.  I've chased this kind of thing down repeatedly through the years.  
It's rare, but some transformations from match.pd create new SSA_NAMEs 
and the various passes need to be prepared to handle that.

Jeff