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 > > > > > + (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> > > > > > >