Re: [PATCH][AArch64][1/3] PR target/84164: Simplify subreg + redundant AND-immediate

[Jeff Law <law at redhat dot com>]

On 03/01/2018 08:19 AM, Kyrill Tkachov wrote:
> Ping.
> 
> Thanks,
> Kyrill
> 
> On 19/02/18 11:35, Kyrill Tkachov wrote:
>> Ping.
>>
>> https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00649.html
>>
>> CC'ing Eric and Jeff as the patch contains a simplify-rtx.c component
>> that I'll need midend approval on.
>>
>> Thanks everyone for your comments so far.
>> Kyrill
>>
>> On 12/02/18 15:18, Kyrill Tkachov wrote:
>>> Hi Richard,
>>>
>>> On 08/02/18 20:29, Richard Sandiford wrote:
>>>> Thanks for doing this.
>>>>
>>>> Kyrill  Tkachov <kyrylo.tkachov@foss.arm.com> writes:
>>>>> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
>>>>> index
>>>>> 2e7aa5c12952ab1a9b49b5adaf23710327e577d3..af06d7502cebac03cefc689b2646874b8397e767
>>>>> 100644
>>>>> --- a/gcc/simplify-rtx.c
>>>>> +++ b/gcc/simplify-rtx.c
>>>>> @@ -6474,6 +6474,18 @@ simplify_subreg (machine_mode outermode, rtx
>>>>> op,
>>>>>         return NULL_RTX;
>>>>>       }
>>>>>   +  /* Simplify (subreg:QI (and:SI (reg:SI) (const_int 0xffff)) 0)
>>>>> +     into (subreg:QI (reg:SI) 0).  */
>>>>> +  scalar_int_mode int_outermode, int_innermode;
>>>>> +  if (!paradoxical_subreg_p (outermode, innermode)
>>>>> +      && is_a <scalar_int_mode> (outermode, &int_outermode)
>>>>> +      && is_a <scalar_int_mode> (innermode, &int_innermode)
>>>>> +      && GET_CODE (op) == AND && CONST_INT_P (XEXP (op, 1))
>>>>> +      && known_eq (subreg_lowpart_offset (outermode, innermode),
>>>>> byte)
>>>>> +      && (~INTVAL (XEXP (op, 1)) & GET_MODE_MASK (int_outermode))
>>>>> == 0
>>>>> +      && validate_subreg (outermode, innermode, XEXP (op, 0), byte))
>>>>> +    return gen_rtx_SUBREG (outermode, XEXP (op, 0), byte);
>>>>> +
>>>>>     /* A SUBREG resulting from a zero extension may fold to zero if
>>>>>        it extracts higher bits that the ZERO_EXTEND's source bits.  */
>>>>>     if (GET_CODE (op) == ZERO_EXTEND && SCALAR_INT_MODE_P (innermode))
>>>> I think it'd be better to do this in simplify_truncation (shared
>>>> by the subreg code and the TRUNCATE code).  The return would then
>>>> be simplify_gen_unary (TRUNCATE, ...), which will become a subreg
>>>> if TRULY_NOOP_TRUNCATION.
>>>
>>> Thanks, that does look cleaner.
>>> Bootstrapped and tested on arm-none-linux-gnueabihf,
>>> aarch64-none-linux-gnu and x86_64-unknown-linux-gnu.
>>> The other two patches are still needed to address the fallout.
>>>
>>> Is this ok?
>>>
>>> Thanks,
>>> Kyrill
>>>
>>> 2018-02-12  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>     PR target/84164
>>>     * simplify-rtx.c (simplify_truncation): Simplify truncation of
>>> masking
>>>     operation.
>>>     * config/aarch64/aarch64.md (*aarch64_reg_<mode>3_neg_mask2):
>>>     Use simplify_gen_unary creating a SUBREG.
>>>     (*aarch64_reg_<mode>3_minus_mask): Likewise.
>>>     (*aarch64_<optab>_reg_di3_mask2): Use const_int_operand predicate
>>>     for operand 3.
>>>
>>> 2018-02-12  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>     PR target/84164
>>>     * gcc.c-torture/compile/pr84164.c: New test.
Sorry.  I suspect I dropped this from my inbox when it had the AArch64
marker -- I didn't realize it had a target independent component.

The simplify-rtx bits are fine.  The version in simplify_truncation is
much better than the original in simplify_subreg (which I think needed
to verify that you were looking at the lowpart before optimizing).

jeff
>>
>