> On 11 Oct 2024, at 12:28, Richard Sandiford <richard.sandif...@arm.com> wrote:
>
> External email: Use caution opening links or attachments
>
>
> Kyrylo Tkachov <ktkac...@nvidia.com> writes:
>> Hi all,
>>
>> In the testcase from patch [2/2] we want to match a vector rotate operation
>> from
>> an IOR of left and right shifts by immediate. simplify-rtx has code for just
>> that but it looks like it's prepared to do handle only scalar operands.
>> In practice most of the code works for vector modes as well except the shift
>> amounts are checked to be CONST_INT rather than vector constants that we have
>> here. This is easily extended by using unwrap_const_vec_duplicate to extract
>> the repeating constant shift amount.
>
> FWIW, shifting a vector by a scalar is valid rtl (at least AIUI), so the
> current code does handle that case. But I agree it's missing shifting a
> vector by a vector.
>
> I suppose a fancy version would be to check the rotate condition for each
> individual element of the vector shift amount. Checking the duplicate
> case is definitely a good (and strict) improvement over the status quo
> though.
>
>> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
>> index e8e60404ef6..7ff14594daa 100644
>> --- a/gcc/simplify-rtx.cc
>> +++ b/gcc/simplify-rtx.cc
>> @@ -3477,12 +3477,16 @@ simplify_context::simplify_binary_operation_1
>> (rtx_code code,
>> }
>>
>> if (GET_CODE (opleft) == ASHIFT && GET_CODE (opright) == LSHIFTRT
>> - && rtx_equal_p (XEXP (opleft, 0), XEXP (opright, 0))
>> - && CONST_INT_P (XEXP (opleft, 1))
>> - && CONST_INT_P (XEXP (opright, 1))
>> - && (INTVAL (XEXP (opleft, 1)) + INTVAL (XEXP (opright, 1))
>> + && rtx_equal_p (XEXP (opleft, 0), XEXP (opright, 0)))
>> + {
>> + rtx leftcst = unwrap_const_vec_duplicate (XEXP (opleft, 1));
>> + rtx rightcst = unwrap_const_vec_duplicate (XEXP (opright, 1));
>> +
>> + if (CONST_INT_P (leftcst) && CONST_INT_P (rightcst)
>> + && (INTVAL (leftcst) + INTVAL (rightcst)
>> == GET_MODE_UNIT_PRECISION (mode)))
>
> Nit: looks like some reindentation might be missing here.
>
>> - return gen_rtx_ROTATE (mode, XEXP (opright, 0), XEXP (opleft, 1));
>> + return gen_rtx_ROTATE (mode, XEXP (opright, 0), XEXP (opleft, 1));
>> + }
>
> Looks good. So referring back to the above, vector shifts will retain a
> scalar shift amount if they started with a scalar shift amount, and a
> vector shift amount if they started with a vector shift amount.
>
> OK with formatting tweak, thanks.Thanks for the context and the discussion on the series. I’ll push the attached (with the fixed indentation). Kyrill
0001-PR-117048-simplify-rtx-Extend-x-C1-X-C2-ROTATE-trans-v2.patch
Description: 0001-PR-117048-simplify-rtx-Extend-x-C1-X-C2-ROTATE-trans-v2.patch
