> On 13 Nov 2024, at 2:49 PM, Richard Biener <rguent...@suse.de> wrote:
>
> External email: Use caution opening links or attachments
>
>
> On Wed, 13 Nov 2024, Soumya AR wrote:
>
>>
>>
>>> On 12 Nov 2024, at 6:19 PM, Richard Biener <rguent...@suse.de> wrote:
>>>
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Mon, 11 Nov 2024, Soumya AR wrote:
>>>
>>>> Hi Richard,
>>>>
>>>>> On 7 Nov 2024, at 6:10 PM, Richard Biener <rguent...@suse.de> wrote:
>>>>>
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On Tue, 5 Nov 2024, Soumya AR wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>>> On 29 Oct 2024, at 7:16 PM, Richard Biener <rguent...@suse.de> wrote:
>>>>>>>
>>>>>>> External email: Use caution opening links or attachments
>>>>>>>
>>>>>>>
>>>>>>> On Mon, 28 Oct 2024, Soumya AR wrote:
>>>>>>>
>>>>>>>> This patch transforms the following POW calls to equivalent LDEXP
>>>>>>>> calls, as
>>>>>>>> discussed in PR57492:
>>>>>>>>
>>>>>>>> powi (2.0, i) -> ldexp (1.0, i)
>>>>>>>>
>>>>>>>> a * powi (2.0, i) -> ldexp (a, i)
>>>>>>>>
>>>>>>>> 2.0 * powi (2.0, i) -> ldexp (1.0, i + 1)
>>>>>>>>
>>>>>>>> pow (powof2, i) -> ldexp (1.0, i * log2 (powof2))
>>>>>>>>
>>>>>>>> powof2 * pow (2, i) -> ldexp (1.0, i + log2 (powof2))
>>>>>>>
>>>>>>> For the multiplication cases why not handle powof2 * ldexp (1., i)
>>>>>>> to ldexp (1., i + log2 (powof2)) and a * ldexp (1., i) -> ldexp (a, i)
>>>>>>> instead? exp2 * ldexp (1., i) is another candidate.
>>>>>>>
>>>>>>> So please split out the multiplication parts.
>>>>>>>
>>>>>>> + /* Simplify pow (powof2, i) to ldexp (1, i * log2 (powof2)). */
>>>>>>>
>>>>>>> the below pattern handles POWI, not POW.
>>>>>>>
>>>>>>> + (simplify
>>>>>>> + (POWI REAL_CST@0 @1)
>>>>>>> + (with { HOST_WIDE_INT tmp = 0;
>>>>>>> + tree integer_arg1 = NULL_TREE; }
>>>>>>> + (if (integer_valued_real_p (@0)
>>>>>>> + && real_isinteger (&TREE_REAL_CST (@0), &tmp)
>>>>>>> + && integer_pow2p (integer_arg1 = build_int_cst
>>>>>>> (integer_type_node,
>>>>>>> tmp)))
>>>>>>>
>>>>>>> && tmp > 0
>>>>>>> && pow2p_hwi (tmp)
>>>>>>>
>>>>>>> + (LDEXP { build_one_cst (type); }
>>>>>>> + (mult @1 { build_int_cst (integer_type_node,
>>>>>>> + tree_log2 (integer_arg1)); })))))
>>>>>>>
>>>>>>> build_int_cst (integer_type_node, exact_log2 (tmp))
>>>>>>>
>>>>>>> + /* Simplify powi (2.0, i) to ldexp (1, i). */
>>>>>>> + (simplify
>>>>>>> + (POWI REAL_CST@0 @1)
>>>>>>> + (if (real_equal (TREE_REAL_CST_PTR (@0), &dconst2))
>>>>>>> + (LDEXP { build_one_cst (type); } @1)))
>>>>>>> +
>>>>>>>
>>>>>>> You'll have a duplicate pattern here, instead merge them. 2.0
>>>>>>> is power-of-two so I wonder why the pattern is needed.
>>>>>>
>>>>>> Thanks for the feedback!
>>>>>>
>>>>>> I've merged the extra case that was specifically checking for 2.0.
>>>>>>
>>>>>> Like you suggested, I also added two ldexp specific transforms:
>>>>>>
>>>>>> • powof2 * ldexp (x, i) -> ldexp (x, i + log2 (powof2))
>>>>>> • a * ldexp(1., i) -> ldexp (a, i)
>>>>>>
>>>>>> Regarding your suggestion to also fold exp2, a conversion like
>>>>>> exp2 (x) -> ldexp (1., x) or exp2 (x) * ldexp (y, i) -> ldexp (y, i + x)
>>>>>> is a
>>>>>> bit tricky because we'd have to cast it to an integer before passing it
>>>>>> to
>>>>>> ldexp.
>>>>>>
>>>>>> real_to_integer only works for constants, which isn't helpful here as
>>>>>> exp2 (CST)
>>>>>> becomes a power of 2 anyway and matches with the above patterns.
>>>>>>
>>>>>> We'll have to explicitly convert it for non constants and I'm not sure
>>>>>> if that
>>>>>> is worth it for this patch.
>>>>>>
>>>>>> Let me know what you think.
>>>>>
>>>>> + (simplify
>>>>> + (mult:c REAL_CST@0 (POWI REAL_CST@1 @2))
>>>>> + (with { HOST_WIDE_INT tmp = 0;
>>>>> + tree integer_arg1 = NULL_TREE; }
>>>>> + (if (integer_valued_real_p (@0)
>>>>> + && real_equal (TREE_REAL_CST_PTR (@1), &dconst2)
>>>>> + && real_isinteger (&TREE_REAL_CST (@0), &tmp)
>>>>> + && integer_pow2p (integer_arg1 = build_int_cst (integer_type_node,
>>>>> tmp)))
>>>>>
>>>>> as said you can use tmp > 0 && pow2p_hwi (tmp) instead of
>>>>> integer_pow2p and build_int_cst. This applies to all patterns.
>>>>
>>>> Thanks for the feedback! Updated the patch with this modification.
>>>>
>>>>> + (if (integer_valued_real_p (@0)
>>>>> + && real_isinteger (&TREE_REAL_CST (@0), &tmp)
>>>>>
>>>>> also is a redundant check, real_isinteger gives you the answer already.
>>>>>
>>>>> + (mult @1 {build_int_cst (integer_type_node,
>>>>> + tree_log2 (integer_arg1)); })))))
>>>>>
>>>>> and use exact_log2 (tmp) instead of tree_log2 (integer_arg1).
>>>>
>>>> Fixed.
>>>>
>>>>>
>>>>> + /* Simplify a * powi (2.0, i) to ldexp (a, i). */
>>>>> + (simplify
>>>>> + (mult:c @0 (POWI REAL_CST@1 @2))
>>>>> + (if (real_equal (TREE_REAL_CST_PTR (@1), &dconst2))
>>>>> + (LDEXP @0 @2)))
>>>>> ...
>>>>> + /* Simplify powi (2.0, i) to ldexp (1, i). */
>>>>> + (simplify
>>>>> + (POWI REAL_CST@0 @1)
>>>>> + (if (real_equal (TREE_REAL_CST_PTR (@0), &dconst2))
>>>>> + (LDEXP { build_one_cst (type); } @1)))
>>>>>
>>>>> you can expect the first to never hit as we should replace
>>>>> a * powi (2.0, i) with a * ldexp (1, i) first. So you can
>>>>> drop the first pattern.
>>>>
>>>> Makes sense, apologies for the oversight. Fixed this.
>>>
>>> + /* Simplify powi (2.0, i) to ldexp (1, i). */
>>> + (simplify
>>> + (POWI REAL_CST@0 @1)
>>> + (if (real_equal (TREE_REAL_CST_PTR (@0), &dconst2))
>>> + (LDEXP { build_one_cst (type); } @1)))
>>>
>>> why do you need this? The powi (powof2, i) to ldexp (1, i * log2
>>> (powof2)) pattern should handle this already?
>>>
>>> + /* Simplify powof2 * powi (2, i) to ldexp (1, i + log2 (powof2)). */
>>> + (simplify
>>> + (mult:c REAL_CST@0 (POWI REAL_CST@1 @2))
>>> + (with { HOST_WIDE_INT tmp = 0; }
>>> + (if (real_isinteger (&TREE_REAL_CST (@0), &tmp)
>>> + && tmp > 0 && pow2p_hwi (tmp))
>>> + (LDEXP { build_one_cst (type); }
>>> + (plus {build_int_cst (integer_type_node, exact_log2 (tmp)); }
>>> @2)))))
>>> +
>>> + /* Simplify powi (powof2, i) to ldexp (1, i * log2 (powof2)). */
>>> + (simplify
>>> + (POWI REAL_CST@0 @1)
>>> + (with { HOST_WIDE_INT tmp = 0; }
>>> + (if (real_isinteger (&TREE_REAL_CST (@0), &tmp)
>>> + && tmp > 0 && pow2p_hwi (tmp))
>>> + (LDEXP { build_one_cst (type); }
>>> + (mult @1 {build_int_cst (integer_type_node,
>>> + exact_log2 (tmp)); })))))
>>>
>>> with the second pattern you shouldn't see powof2 * powi (2, i)
>>> since powi (2, i) will be simplified to ldexp already, so the
>>> powof2 * powi (2, i) simplification should not be necessary.
>>>
>>> Otherwise looks good now.
>>
>> Removed both redundant cases as suggested.
>
> OK.
>
> Thanks,
> Richard.
Committed with 5a674367c6da870184f3bdb7ec110b96aa91bb2b
Thank you!
Best,
Soumya
>
>> Best,
>> Soumya
>>
>>>
>>> Richard.
>>>
>>>> Best,
>>>> Soumya
>>>>
>>>>> Richard.
>>>>>
>>>>>
>>>>>> Best,
>>>>>> Soumya
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Richard.
>>>>>>>
>>>>>>>>
>>>>>>>> This is especially helpful for SVE architectures as LDEXP calls can be
>>>>>>>> implemented using the FSCALE instruction, as seen in the following
>>>>>>>> patch:
>>>>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2024-September/664160.html
>>>>>>>>
>>>>>>>> SPEC2017 was run with this patch, while there are no noticeable
>>>>>>>> improvements,
>>>>>>>> there are no non-noise regressions either.
>>>>>>>>
>>>>>>>> The patch was bootstrapped and regtested on aarch64-linux-gnu, no
>>>>>>>> regression.
>>>>>>>> OK for mainline?
>>>>>>>>
>>>>>>>> Signed-off-by: Soumya AR <soum...@nvidia.com>
>>>>>>>>
>>>>>>>> gcc/ChangeLog:
>>>>>>>> PR target/57492
>>>>>>>> * match.pd: Added patterns to fold certain calls to pow to ldexp.
>>>>>>>>
>>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>> PR target/57492
>>>>>>>> * gcc.dg/tree-ssa/pow-to-ldexp.c: New test.
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Richard Biener <rguent...@suse.de>
>>>>>>> SUSE Software Solutions Germany GmbH,
>>>>>>> Frankenstrasse 146, 90461 Nuernberg, Germany;
>>>>>>> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG
>>>>>>> Nuernberg)
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> Richard Biener <rguent...@suse.de>
>>>>> SUSE Software Solutions Germany GmbH,
>>>>> Frankenstrasse 146, 90461 Nuernberg, Germany;
>>>>> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
>>>>
>>>>
>>>>
>>>
>>> --
>>> Richard Biener <rguent...@suse.de>
>>> SUSE Software Solutions Germany GmbH,
>>> Frankenstrasse 146, 90461 Nuernberg, Germany;
>>> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
>>
>>
>>
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
