Hi Richard,
> On 22 Aug 2024, at 10:34 pm, Richard Biener <richard.guent...@gmail.com>
> wrote:
>
> External email: Use caution opening links or attachments
>
>
> On Wed, Aug 21, 2024 at 12:08 PM Kugan Vivekanandarajah
> <kvivekana...@nvidia.com> wrote:
>>
>> Hi Richard,
>>
>>> On 20 Aug 2024, at 6:09 pm, Richard Biener <richard.guent...@gmail.com>
>>> wrote:
>>>
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Fri, Aug 9, 2024 at 2:39 AM Kugan Vivekanandarajah
>>> <kvivekana...@nvidia.com> wrote:
>>>>
>>>> Thanks for the comments.
>>>>
>>>>> On 2 Aug 2024, at 8:36 pm, Richard Biener <richard.guent...@gmail.com>
>>>>> wrote:
>>>>>
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On Fri, Aug 2, 2024 at 11:20 AM Kugan Vivekanandarajah
>>>>> <kvivekana...@nvidia.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On 1 Aug 2024, at 10:46 pm, Richard Biener <richard.guent...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> External email: Use caution opening links or attachments
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Aug 1, 2024 at 5:31 AM Kugan Vivekanandarajah
>>>>>>> <kvivekana...@nvidia.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, Jul 29, 2024 at 10:11 AM Andrew Pinski <pins...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> On Mon, Jul 29, 2024 at 12:57 AM Kugan Vivekanandarajah
>>>>>>>>> <kugan...@gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On Thu, Jul 25, 2024 at 10:19 PM Richard Biener
>>>>>>>>>> <richard.guent...@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Jul 25, 2024 at 4:42 AM Kugan Vivekanandarajah
>>>>>>>>>>> <kugan...@gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Jul 23, 2024 at 11:56 PM Richard Biener
>>>>>>>>>>>> <richard.guent...@gmail.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Jul 23, 2024 at 10:27 AM Kugan Vivekanandarajah
>>>>>>>>>>>>> <kugan...@gmail.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Tue, Jul 23, 2024 at 10:35 AM Andrew Pinski
>>>>>>>>>>>>>> <pins...@gmail.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Mon, Jul 22, 2024 at 5:26 PM Kugan Vivekanandarajah
>>>>>>>>>>>>>>> <kvivekana...@nvidia.com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Revised based on the comment and moved it into existing
>>>>>>>>>>>>>>>> patterns as.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> gcc/ChangeLog:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> * match.pd: Extend A CMP 0 ? A : -A into (type)A CMP 0 ? A :
>>>>>>>>>>>>>>>> -A.
>>>>>>>>>>>>>>>> Extend A CMP 0 ? A : -A into (type) A CMP 0 ? A : -A.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> * gcc.dg/tree-ssa/absfloat16.c: New test.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The testcase needs to make sure it runs only for targets that
>>>>>>>>>>>>>>> support
>>>>>>>>>>>>>>> float16 so like:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> /* { dg-require-effective-target float16 } */
>>>>>>>>>>>>>>> /* { dg-add-options float16 } */
>>>>>>>>>>>>>> Added in the attached version.
>>>>>>>>>>>>>
>>>>>>>>>>>>> + /* (type)A >=/> 0 ? A : -A same as abs (A) */
>>>>>>>>>>>>> (for cmp (ge gt)
>>>>>>>>>>>>> (simplify
>>>>>>>>>>>>> - (cnd (cmp @0 zerop) @1 (negate @1))
>>>>>>>>>>>>> - (if (!HONOR_SIGNED_ZEROS (TREE_TYPE(@0))
>>>>>>>>>>>>> - && !TYPE_UNSIGNED (TREE_TYPE(@0))
>>>>>>>>>>>>> - && bitwise_equal_p (@0, @1))
>>>>>>>>>>>>> + (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
>>>>>>>>>>>>> + (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
>>>>>>>>>>>>> + && !TYPE_UNSIGNED (TREE_TYPE (@1))
>>>>>>>>>>>>> + && ((VECTOR_TYPE_P (type)
>>>>>>>>>>>>> + && tree_nop_conversion_p (TREE_TYPE (@0), TREE_TYPE
>>>>>>>>>>>>> (@1)))
>>>>>>>>>>>>> + || (!VECTOR_TYPE_P (type)
>>>>>>>>>>>>> + && (TYPE_PRECISION (TREE_TYPE (@1))
>>>>>>>>>>>>> + <= TYPE_PRECISION (TREE_TYPE (@0)))))
>>>>>>>>>>>>> + && bitwise_equal_p (@1, @2))
>>>>>>>>>>>>>
>>>>>>>>>>>>> I wonder about the bitwise_equal_p which tests @1 against @2 now
>>>>>>>>>>>>> with the convert still applied to @1 - that looks odd. You are
>>>>>>>>>>>>> allowing
>>>>>>>>>>>>> sign-changing conversions but doesn't that change ge/gt behavior?
>>>>>>>>>>>>> Also why are sign/zero-extensions not OK for vector types?
>>>>>>>>>>>> Thanks for the review.
>>>>>>>>>>>> My main motivation here is for _Float16 as below.
>>>>>>>>>>>>
>>>>>>>>>>>> _Float16 absfloat16 (_Float16 x)
>>>>>>>>>>>> {
>>>>>>>>>>>> float _1;
>>>>>>>>>>>> _Float16 _2;
>>>>>>>>>>>> _Float16 _4;
>>>>>>>>>>>> <bb 2> [local count: 1073741824]:
>>>>>>>>>>>> _1 = (float) x_3(D);
>>>>>>>>>>>> if (_1 < 0.0)
>>>>>>>>>>>> goto <bb 3>; [41.00%]
>>>>>>>>>>>> else
>>>>>>>>>>>> goto <bb 4>; [59.00%]
>>>>>>>>>>>> <bb 3> [local count: 440234144]:\
>>>>>>>>>>>> _4 = -x_3(D);
>>>>>>>>>>>> <bb 4> [local count: 1073741824]:
>>>>>>>>>>>> # _2 = PHI <_4(3), x_3(D)(2)>
>>>>>>>>>>>> return _2;
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> This is why I added bitwise_equal_p test of @1 against @2 with
>>>>>>>>>>>> TYPE_PRECISION checks.
>>>>>>>>>>>> I agree that I will have to check for sign-changing conversions.
>>>>>>>>>>>>
>>>>>>>>>>>> Just to keep it simple, I disallowed vector types. I am not sure if
>>>>>>>>>>>> this would hit vec types. I am happy to handle this if that is
>>>>>>>>>>>> needed.
>>>>>>>>>>>
>>>>>>>>>>> I think with __builtin_convertvector you should be able to construct
>>>>>>>>>>> a testcase that does
>>>>>>>>>> Thanks.
>>>>>>>>>>
>>>>>>>>>> For the pattern,
>>>>>>>>>> ```
>>>>>>>>>> /* A >=/> 0 ? A : -A same as abs (A) */
>>>>>>>>>> (for cmp (ge gt)
>>>>>>>>>> (simplify
>>>>>>>>>> (cnd (cmp @0 zerop) @1 (negate @1))
>>>>>>>>>> (if (!HONOR_SIGNED_ZEROS (TREE_TYPE(@0))
>>>>>>>>>> && !TYPE_UNSIGNED (TREE_TYPE(@0))
>>>>>>>>>> && bitwise_equal_p (@0, @1))
>>>>>>>>>> (if (TYPE_UNSIGNED (type))
>>>>>>>>>> (absu:type @0)
>>>>>>>>>> (abs @0)))))
>>>>>>>>>> ```
>>>>>>>>>> the vector type doesn't seem right. For example, if we have a 4
>>>>>>>>>> element vector with some negative and positive, I don't think it
>>>>>>>>>> makes sense. Also, we dont seem to generate (cmp @0 zerop). Am I
>>>>>>>>>> missing it completely?
>>>>>>>>>
>>>>>>>>> Looks like I missed adding some vector testcases anyways here is one
>>>>>>>>> to get this, note it is C++ due to the C front-end not support `?:`
>>>>>>>>> for vectors yet (there is a patch).
>>>>>>>>> ```
>>>>>>>>> #define vect8 __attribute__((vector_size(8)))
>>>>>>>>> vect8 int f(vect8 int a)
>>>>>>>>> {
>>>>>>>>> vect8 int na = -a;
>>>>>>>>> return (a > 0) ? a : na;
>>>>>>>>> }
>>>>>>>>> ```
>>>>>>>>> At -O2 before forwprop1, we have:
>>>>>>>>> ```
>>>>>>>>> vector(2) intD.9 a_2(D) = aD.2796;
>>>>>>>>> vector(2) intD.9 naD.2799;
>>>>>>>>> vector(2) <signed-boolean:32> _1;
>>>>>>>>> vector(2) intD.9 _4;
>>>>>>>>>
>>>>>>>>> na_3 = -a_2(D);
>>>>>>>>> _1 = a_2(D) > { 0, 0 };
>>>>>>>>> _4 = VEC_COND_EXPR <_1, a_2(D), na_3>;
>>>>>>>>> ```
>>>>>>>>> And forwprop using match is able to do:
>>>>>>>>> ```
>>>>>>>>> Applying pattern match.pd:6306, gimple-match-10.cc:19843
>>>>>>>>> gimple_simplified to _4 = ABS_EXPR <a_2(D)>;
>>>>>>>>> Removing dead stmt:_1 = a_2(D) > { 0, 0 };
>>>>>>>>> Removing dead stmt:na_3 = -a_2(D);
>>>>>>>>> ```
>>>>>>>>> (replace int with float and add -fno-signed-zeros you can get the
>>>>>>>>> ABS also).
>>>>>>>>>
>>>>>>>>> Note comparisons with vector types always generate a vector boolean
>>>>>>>>> type. So cond_expr will never show up with a vector comparison; only
>>>>>>>>> vec_cond.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for the information. I have revised the patch by adding test
>>>>>>>> case for the vector type. I will look at removing the VEC_COND_EXPR as
>>>>>>>> a follow up.
>>>>>>>
>>>>>>> This is likely a major task so do not get you distracted - I just
>>>>>>> mentioned it.
>>>>>>>
>>>>>>>> This is still rejected by the type checker.
>>>>>>>> v2hi absvect2 (v2hi x, int i) {
>>>>>>>> v2hi neg = -x;
>>>>>>>> v2si tmp = __builtin_convertvector (x, v2si);
>>>>>>>> return (tmp > 0) ? x : neg;
>>>>>>>> }
>>>>>>>> as:
>>>>>>>> error: incompatible vector types in conditional expression:
>>>>>>>> '__vector(2) int', 'v2hi' {aka '__vector(2) short int'} and 'v2hi'
>>>>>>>> {aka '__vector(2) short int'}
>>>>>>>> 8 | return (tmp > 0) ? x : neg;
>>>>>>>> | ~~~~~~~~~~^~~~~~~~~
>>>>>>>>
>>>>>>>> Also
>>>>>>>>
>>>>>>>> - (absu:type @0)
>>>>>>>> - (abs @0)))))
>>>>>>>> + (absu:type @1)
>>>>>>>> + (abs @1)))))
>>>>>>>>
>>>>>>>> Changing the @1 to @2 is resulting in failures. Existing tests like
>>>>>>>> the following would be optimized away in this case.
>>>>>>>> ```
>>>>>>>> unsigned abs_with_convert0 (int x)
>>>>>>>> {
>>>>>>>> unsigned int y = x;
>>>>>>>>
>>>>>>>> if (x < 0)
>>>>>>>> y = -y;
>>>>>>>>
>>>>>>>> return y;
>>>>>>>> }
>>>>>>>
>>>>>>> How so - the TYPE_UNSIGNED tests should make the pattern not trigger
>>>>>>> here?
>>>>>>>
>>>>>>>> ```
>>>>>>>> This follows the same intent as the original pattern.
>>>>>>>>
>>>>>>>> Bootstrapped and regression tested on aarch64-linux-gnu. Is this OK
>>>>>>>> for trunk.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> - (cnd (cmp @0 zerop) @1 (negate @1))
>>>>>>> - (if (!HONOR_SIGNED_ZEROS (TREE_TYPE(@0))
>>>>>>> - && !TYPE_UNSIGNED (TREE_TYPE(@0))
>>>>>>> - && bitwise_equal_p (@0, @1))
>>>>>>> + (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
>>>>>>> + (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
>>>>>>> + && !TYPE_UNSIGNED (TREE_TYPE (@1))
>>>>>>> + && !TYPE_UNSIGNED (TREE_TYPE (@0))
>>>>>>>
>>>>>>> I think the outer type could be allowed signed if ...
>>>>>>>
>>>>>>> + && ((VECTOR_TYPE_P (TREE_TYPE (@0))
>>>>>>> + && VECTOR_TYPE_P (TREE_TYPE (@1))
>>>>>>> + && known_eq (TYPE_VECTOR_SUBPARTS (TREE_TYPE (@0)),
>>>>>>> + TYPE_VECTOR_SUBPARTS (TREE_TYPE (@1)))
>>>>>>>
>>>>>>> this is always true
>>>>>>>
>>>>>>> + && element_precision (TREE_TYPE (@1))
>>>>>>> + <= element_precision (TREE_TYPE (@0)))
>>>>>>> + || (!VECTOR_TYPE_P (TREE_TYPE (@0))
>>>>>>> + && (TYPE_PRECISION (TREE_TYPE (@1))
>>>>>>> + <= TYPE_PRECISION (TREE_TYPE (@0)))))
>>>>>>>
>>>>>>> ... you make the precision strictly larger. With both unsigned the same
>>>>>>> precision case should have been stripped anyway.
>>>>>>>
>>>>>>> You can use element_precision for both vector and non-vector so I think
>>>>>>> this
>>>>>>> should simplify to just checking element_precision.
>>>>>>>
>>>>>>> + (absu:type @1)
>>>>>>> + (abs @1)))))
>>>>>>>
>>>>>>> I still think this needs to be @2 for type correctness. @1 is only
>>>>>>> bitwise equal,
>>>>>>> it could have different sign. I think we should drop bitwise_equal_p
>>>>>>> with the
>>>>>>> convert now in and instead have a matching constraint.
>>>>>>
>>>>>> Thanks, so this should translate to:
>>>>>>
>>>>>> /* (type)A >=/> 0 ? A : -A same as abs (A) */
>>>>>> (for cmp (ge gt)
>>>>>> (simplify
>>>>>> (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
>>>>>> (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
>>>>>> && !TYPE_UNSIGNED (TREE_TYPE (@1))
>>>>>
>>>>> Oh, I mis-read this as unsigned, so this makes the conversions
>>>>> to always sign-extend which means it's important to ensure
>>>>> the type of @0 is also signed.
>>>>>
>>>>>> && !TYPE_UNSIGNED (TREE_TYPE (@2))
>>>>>> && ((element_precision (TREE_TYPE (@1))
>>>>>> < element_precision (TREE_TYPE (@0))
>>>>>> || operand_equal_p (@1, @0)))
>>>>>
>>>>> which means <= might be better then.
>>>>>
>>>>>
>>>>>> && bitwise_equal_p (@1, @2))
>>>>>> (if (TYPE_UNSIGNED (type))
>>>>>> (absu:type @2)
>>>>>> (abs @2)))))
>>>>>>
>>>>>> However with this, I am seeing: Note that the @2 for these are unsigned.
>>>>>
>>>>> I see, so we rely on @1 being the signed equivalent of @2 which might or
>>>>> might not be signed. I guess that indeed we want @2 here.
>>>> Sorry I am not sure I understand this. When @2 is unsigned, ABS_EXPR and
>>>> ABSU_EXPR is not
>>>> For example:
>>>> With
>>>> /* (type)A >=/> 0 ? A : -A same as abs (A) */
>>>> (for cmp (ge gt)
>>>> (simplify
>>>> (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
>>>> (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
>>>> && !TYPE_UNSIGNED (TREE_TYPE (@0))
>>>> && !TYPE_UNSIGNED (TREE_TYPE (@1))
>>>> && element_precision (TREE_TYPE (@1))
>>>> <= element_precision (TREE_TYPE (@0))
>>>> && bitwise_equal_p (@1, @2))
>>>> (if (TYPE_UNSIGNED (type))
>>>> (absu:type @2)
>>>> (abs @2)))))
>>>>
>>>> The test case below becomes:
>>>>
>>>> unsigned abs_with_convert0 (int x)
>>>> {
>>>> unsigned int y = x;
>>>>
>>>> if (x < 0)
>>>> y = -y;
>>>>
>>>> return y;
>>>> }
>>>>
>>>> unsigned int abs_with_convert0 (int x)
>>>> {
>>>> unsigned int y;
>>>>
>>>> <bb 2> :
>>>> y_3 = (unsigned int) x_2(D);
>>>> if (x_2(D) < 0)
>>>> goto <bb 3>; [INV]
>>>> else
>>>> goto <bb 4>; [INV]
>>>>
>>>> <bb 3> :
>>>> y_4 = -y_3;
>>>>
>>>> <bb 4> :
>>>> # y_1 = PHI <y_3(2), y_4(3)>
>>>> return y_1;
>>>>
>>>> }
>>>>
>>>> To:
>>>>
>>>>
>>>> unsigned int abs_with_convert0 (int x)
>>>> {
>>>> unsigned int y;
>>>> unsigned int _5;
>>>>
>>>> <bb 2> :
>>>> y_3 = (unsigned int) x_2(D);
>>>> _5 = ABSU_EXPR <y_3>;
>>>> return _5;
>>>>
>>>> }
>>>>
>>>> This is an invalid gimple. Are you suggesting that we should also check if
>>>> @2 is not UNSIGNED? If that is what you want, couple of test cases in the
>>>> test suite including phi-opt-37.c would fail.
>>>
>>> Trying to swap in the discussion after vacation ... iff @2 might be
>>> unsigned then you'd need
>>> a conversion to signed to make 'abs' make sense.
>>>
>>>> (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
>>>> (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
>>>> && !TYPE_UNSIGNED (TREE_TYPE (@0))
>>>> && !TYPE_UNSIGNED (TREE_TYPE (@1))
>>>
>>> so this doesn't put any constraints on the signedness of @2
>>>
>>> What kind of extensions are valid on @1? I think this warrants a
>>> comment. I think in the end the compare should be signed to
>>> fit 'abs' semantics, but a zero-extension (unsigned @1) should be
>>> OK? So why constrain the sign of @1?
>> I thing it is the conversion to signed (sign extend) that we want to
>> support, But for cases like:
>> signed abs_with_convert1 (unsigned x)
>> {
>> int y = x;
>>
>> if (y < 0)
>> y = -x;
>>
>> return y;
>> }
>> @2 is unsigned.
>>
>>>> && element_precision (TREE_TYPE (@1))
>>>> <= element_precision (TREE_TYPE (@0))
>>>> && bitwise_equal_p (@1, @2))
>>>> (if (TYPE_UNSIGNED (type))
>>>> (absu:type @2)
>>>> (abs @2)))))
>>>
>>> Of course then you'd want to convert @2 to signed?
>>
>> We have some test cases where @2 is unsigned where original pattern triggers
>> and will not with the new pattern. For example gcc.dg/tree-ssa/phi-opt-37.c.
>>
>> Should we create a new pattern as:
>> /* (type)A >=/> 0 ? A : -A same as abs (A) */
>> (for cmp (ge gt)
>> (simplify
>> (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
>> (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
>> && !TYPE_UNSIGNED (TREE_TYPE (@1))
>> && !TYPE_UNSIGNED (TREE_TYPE (@2))
>> && element_precision (TREE_TYPE (@1))
>> <= element_precision (TREE_TYPE (@0))
>> && bitwise_equal_p (@1, @2))
>> (if (TYPE_UNSIGNED (type))
>> (abs @2)
>> (abs @2)))))
>>
>> And leave the old pattern as it is? Otherwise if we decide to use @2, we
>> have to XFAIL these cases unless I am missing spmething.
>
> No, I think we want a single pattern. I meant you possibly need to do
>
> (with
> { tree stype = signed_type_for (TREE_TYPE (@2)); }
> (if (TYPE_UNSIGNED (type))
> (absu (convert:stype @2))
> (abs (convert:stype @2))))
>
> for when @2 is unsigned?
Thanks. I have changed accordingly.
I have also changed gcc.dg/tree-ssa/phi-opt-37.c to check phiopt2 as in phiopt1
we don’t now match because of:
phiopt match-simplify back:
_5 = (signed int) x_2(D);
_7 = ABSU_EXPR <_5>;
result: _7
rejected because early
Bootstrapped and regression tested on aarch64-linux-gnu. Is this OK?
Thanks,
Kugan
>
> Richard.
>
>
>> Thanks,
>> Kugan
>>>
>>>>
>>>> Thanks,
>>>> Kugan
>>>>
>>>>>
>>>>>> Tests that now fail, but worked before (4 tests):
>>>>>>
>>>>>> gcc: gcc.dg/tree-ssa/phi-opt-36.c scan-tree-dump-not phiopt2 "if "
>>>>>> gcc: gcc.dg/tree-ssa/phi-opt-36.c scan-tree-dump-times phiopt1 "if " 2
>>>>>> gcc: gcc.dg/tree-ssa/phi-opt-37.c scan-tree-dump-not phiopt1 "if "gcc:
>>>>>> gcc.dg/tree-ssa/phi-opt-37.c scan-tree-dump-times phiopt1 "ABSU_EXPR <" 2
>>>>>>
>>>>>>
>>>>>> unsigned f2(int A)
>>>>>> {
>>>>>> unsigned t = A;
>>>>>> // A >= 0? A : -A same as abs (A)
>>>>>> if (A >= 0) return t;
>>>>>> return -t;
>>>>>> }
>>>>>>
>>>>>> unsigned abs_with_convert0 (int x)
>>>>>> {
>>>>>> unsigned int y = x;
>>>>>>
>>>>>> if (x < 0)
>>>>>> y = -y;
>>>>>>
>>>>>> return y;
>>>>>> }
>>>>>> unsigned abs_with_convert1 (unsigned x)
>>>>>> {
>>>>>> int y = x;
>>>>>>
>>>>>> if (y < 0)
>>>>>> x = -x;
>>>>>>
>>>>>> return x;
>>>>>> }
>>>>>>
>>>>>> In the original pattern, we have check for !TYPE_UNSIGNED
>>>>>> (TREE_TYPE(@0)) and (absu:type @0)
>>>>>>
>>>>>> Shouldnt that translate !TYPE_UNSIGNED (TREE_TYPE(@1)) and (absu:type
>>>>>> @1) in the new pattern
>>>>>>
>>>>>> Thanks,
>>>>>> Kugan
>>>>>>
>>>>>>>
>>>>>>> Same comments apply to the 2nd pattern update.
>>>>>>>
>>>>>>> Richard.
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Kugan
>>>>>>>>
>>>>>>>> gcc/ChangeLog:
>>>>>>>>
>>>>>>>> * match.pd: Extend A CMP 0 ? A : -A into (type)A CMP 0 ? A : -A.
>>>>>>>> Extend A CMP 0 ? A : -A into (type) A CMP 0 ? A : -A.
>>>>>>>>
>>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>>
>>>>>>>> * g++.dg/absvect.C: New test.
>>>>>>>> * gcc.dg/tree-ssa/absfloat16.c: New test.
>>>>>>>>
>>>>>>>>
>>>>>>>> signed-off-by: Kugan Vivekanandarajah <kvivekana...@nvidia.com>
>>>>>>>>
>>>>>>>>
>>>>>>>> Note I think we should be able to merge VEC_COND_EXPR and COND_EXPR
>>>>>>>> codes by instead looking whether the condition is vector or scalar.
>>>>>>>> Fallout
>>>>>>>> might be a bit too big, but it might be a thing to try.
>>>>>>>>
>>>>>>>> Richard.
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Andrew Pinski
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Kugan
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> + (absu:type @1)
>>>>>>>>>>>>> + (abs @1)))))
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think this should use @2 now.
>>>>>>>>>>>> I will change this.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Kugan
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>> Kugan
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> (like what is in gcc.dg/c11-floatn-3.c and others).
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Other than that it looks good but I can't approve it.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> Andrew Pinski
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Signed-off-by: Kugan Vivekanandarajah <kvivekana...@nvidia.com>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Bootstrapped and regression test on aarch64-linux-gnu. Is this
>>>>>>>>>>>>>>>> OK for trunk?
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Kugan
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> ________________________________
>>>>>>>>>>>>>>>> From: Andrew Pinski <pins...@gmail.com>
>>>>>>>>>>>>>>>> Sent: Monday, 15 July 2024 5:30 AM
>>>>>>>>>>>>>>>> To: Kugan Vivekanandarajah <kvivekana...@nvidia.com>
>>>>>>>>>>>>>>>> Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>;
>>>>>>>>>>>>>>>> richard.guent...@gmail.com <richard.guent...@gmail.com>
>>>>>>>>>>>>>>>> Subject: Re: [PATCH] MATCH: add abs support for half float
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Sun, Jul 14, 2024 at 1:12 AM Kugan Vivekanandarajah
>>>>>>>>>>>>>>>> <kvivekana...@nvidia.com> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> This patch extends abs detection in matched for half float.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Bootstrapped and regression test on aarch64-linux-gnu. Is
>>>>>>>>>>>>>>>>> this OK for trunk?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> This is basically this pattern:
>>>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>>>> /* A >=/> 0 ? A : -A same as abs (A) */
>>>>>>>>>>>>>>>> (for cmp (ge gt)
>>>>>>>>>>>>>>>> (simplify
>>>>>>>>>>>>>>>> (cnd (cmp @0 zerop) @1 (negate @1))
>>>>>>>>>>>>>>>> (if (!HONOR_SIGNED_ZEROS (TREE_TYPE(@0))
>>>>>>>>>>>>>>>> && !TYPE_UNSIGNED (TREE_TYPE(@0))
>>>>>>>>>>>>>>>> && bitwise_equal_p (@0, @1))
>>>>>>>>>>>>>>>> (if (TYPE_UNSIGNED (type))
>>>>>>>>>>>>>>>> (absu:type @0)
>>>>>>>>>>>>>>>> (abs @0)))))
>>>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> except extended to handle an optional convert. Why didn't you
>>>>>>>>>>>>>>>> just
>>>>>>>>>>>>>>>> extend the above pattern to handle the convert instead? Also I
>>>>>>>>>>>>>>>> think
>>>>>>>>>>>>>>>> you have an issue with unsigned types with the comparison.
>>>>>>>>>>>>>>>> Also you should extend the -abs(A) pattern right below it in a
>>>>>>>>>>>>>>>> similar fashion.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Andrew Pinski
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> gcc/ChangeLog:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> * match.pd: Add pattern to convert (type)A >=/> 0 ? A : -A
>>>>>>>>>>>>>>>>> into abs (A) for half float.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> * gcc.dg/tree-ssa/absfloat16.c: New test.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Signed-off-by: Kugan Vivekanandarajah
>>>>>>>>>>>>>>>>> <kvivekana...@nvidia.com>
0001-abshalffloat-v5.patch
Description: 0001-abshalffloat-v5.patch
