On Wed, Sep 25, 2024 at 12:12 PM Kugan Vivekanandarajah
<kvivekana...@nvidia.com> wrote:
>
> Hi Richard,
>
> > On 24 Sep 2024, at 6:16 pm, Richard Biener <richard.guent...@gmail.com>
> > wrote:
> >
> > External email: Use caution opening links or attachments
> >
> >
> > On Mon, Sep 23, 2024 at 10:52 AM Kugan Vivekanandarajah
> > <kvivekana...@nvidia.com> wrote:
> >>
> >> Hi Richard,
> >>
> >>> On 20 Sep 2024, at 8:11 pm, Richard Biener <richard.guent...@gmail.com>
> >>> wrote:
> >>>
> >>> External email: Use caution opening links or attachments
> >>>
> >>>
> >>> On Fri, Sep 20, 2024 at 10:23 AM Kugan Vivekanandarajah
> >>> <kvivekana...@nvidia.com> wrote:
> >>>>
> >>>> Hi Richard,
> >>>>
> >>>>> On 17 Sep 2024, at 7:36 pm, Richard Biener <richard.guent...@gmail.com>
> >>>>> wrote:
> >>>>>
> >>>>> External email: Use caution opening links or attachments
> >>>>>
> >>>>>
> >>>>> On Tue, Sep 17, 2024 at 10:31 AM Kugan Vivekanandarajah
> >>>>> <kvivekana...@nvidia.com> wrote:
> >>>>>>
> >>>>>> Hi Richard,
> >>>>>>
> >>>>>>> On 10 Sep 2024, at 9:33 pm, Richard Biener
> >>>>>>> <richard.guent...@gmail.com> wrote:
> >>>>>>>
> >>>>>>> External email: Use caution opening links or attachments
> >>>>>>>
> >>>>>>>
> >>>>>>> On Thu, Sep 5, 2024 at 3:19 AM Kugan Vivekanandarajah
> >>>>>>> <kvivekana...@nvidia.com> wrote:
> >>>>>>>>
> >>>>>>>> Thanks for the explanation.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On 2 Sep 2024, at 9:47 am, Andrew Pinski <pins...@gmail.com> wrote:
> >>>>>>>>>
> >>>>>>>>> External email: Use caution opening links or attachments
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On Sun, Sep 1, 2024 at 4:27 PM Kugan Vivekanandarajah
> >>>>>>>>> <kvivekana...@nvidia.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> Hi Andrew.
> >>>>>>>>>>
> >>>>>>>>>>> On 28 Aug 2024, at 2:23 pm, Andrew Pinski <pins...@gmail.com>
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> External email: Use caution opening links or attachments
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On Tue, Aug 27, 2024 at 8:54 PM Kugan Vivekanandarajah
> >>>>>>>>>>> <kvivekana...@nvidia.com> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> Hi Richard,
> >>>>>>>>>>>>
> >>>>>>>>>>>> Thanks for the reply.
> >>>>>>>>>>>>
> >>>>>>>>>>>>> On 27 Aug 2024, at 7:05 pm, Richard Biener
> >>>>>>>>>>>>> <richard.guent...@gmail.com> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> External email: Use caution opening links or attachments
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On Tue, Aug 27, 2024 at 8:23 AM Kugan Vivekanandarajah
> >>>>>>>>>>>>> <kvivekana...@nvidia.com> wrote:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> 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
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> ? I don't understand what you are saying.
> >>>>>>>>>>>>
> >>>>>>>>>>>> This is the dump from phiopt. Even though it is matching,
> >>>>>>>>>>>> /* Return TRUE if SEQ/OP pair should be allowed during early
> >>>>>>>>>>>> phiopt.
> >>>>>>>>>>>> Currently this is to allow MIN/MAX and ABS/NEGATE and constants.
> >>>>>>>>>>>> */
> >>>>>>>>>>>> static bool
> >>>>>>>>>>>> phiopt_early_allow (gimple_seq &seq, gimple_match_op &op) is
> >>>>>>>>>>>> rejecting it, as the dump shows.
> >>>>>>>>>>>
> >>>>>>>>>>> So I looked into this slightly. I think the match patterns are
> >>>>>>>>>>> greedy
> >>>>>>>>>>> when it comes to optional converts, in that match tries to match
> >>>>>>>>>>> with
> >>>>>>>>>>> convert first and for integer types but bitwise_equal_p will
> >>>>>>>>>>> handle a
> >>>>>>>>>>> nop_convert here.
> >>>>>>>>>>> So for scalar integer types, maybe skip `TYPE_PRECISION (TREE_TYPE
> >>>>>>>>>>> (@1)) == TYPE_PRECISION (TREE_TYPE (@0))` as that is handled via
> >>>>>>>>>>> bitwise_equal_p call and we don't want an extra convert being
> >>>>>>>>>>> added by
> >>>>>>>>>>> the simplification
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> phiopt_early_allow only allow single instruction sequences. But
> >>>>>>>>>> since we now use @2 instead of @1, we will need the conversion.
> >>>>>>>>>> Sorry I don’t understand the fix you want.
> >>>>>>>>>
> >>>>>>>>> The convert expression is already existing before the match.
> >>>>>>>>> ```
> >>>>>>>>> y_3 = (int) x_2(D);
> >>>>>>>>> if (y_3 < 0)
> >>>>>>>>> goto <bb 3>; [INV]
> >>>>>>>>> else
> >>>>>>>>> goto <bb 4>; [INV]
> >>>>>>>>>
> >>>>>>>>> <bb 3> :
> >>>>>>>>> x_4 = -x_2(D);
> >>>>>>>>>
> >>>>>>>>> <bb 4> :
> >>>>>>>>> # x_1 = PHI <x_2(D)(2), x_4(3)>
> >>>>>>>>> return x_1;
> >>>>>>>>> ```
> >>>>>>>>>
> >>>>>>>>> Adding a new convert via match and simplify is not useful and is the
> >>>>>>>>> cause of the issue as I mentioned
> >>>>>>>>>
> >>>>>>>>> So you should add:
> >>>>>>>>> ```
> >>>>>>>>> (if (INTEGRAL_TYPE (type)
> >>>>>>>>> && element_precision (TREE_TYPE (@1))
> >>>>>>>>> == element_precision (TREE_TYPE (@0)))
> >>>>>>>>> (if (TYPE_UNSIGNED (type))
> >>>>>>>>> (absu:type @0)
> >>>>>>>>> (abs @0))
> >>>>>>>>> ```
> >>>>>>>>> So you reuse the convert expression that was there previously and
> >>>>>>>>> the
> >>>>>>>>> testcase will just work.
> >>>>>>>>
> >>>>>>>> Here is the amended patch. Bootstrapped and regression tested on
> >>>>>>>> aarch64-linux-gnu. Is this OK for trunk?
> >>>>>>>
> >>>>>>> + /* Handle nop convert first to prevent additional
> >>>>>>> + convertion. */
> >>>>>>> + (if (INTEGRAL_TYPE_P (type)
> >>>>>>> + && element_precision (TREE_TYPE (@1))
> >>>>>>> + == element_precision (TREE_TYPE (@0)))
> >>>>>>> + (if (TYPE_UNSIGNED (type))
> >>>>>>> + (absu:type @0)
> >>>>>>> + (abs @0))
> >>>>>>> + (if (TYPE_UNSIGNED (type))
> >>>>>>> + (if (TYPE_UNSIGNED (TREE_TYPE (@2)))
> >>>>>>> + (absu (convert:stype @2))
> >>>>>>> + (absu @2))
> >>>>>>> + (if (TYPE_UNSIGNED (TREE_TYPE (@2)))
> >>>>>>> + (abs (convert:stype @2))
> >>>>>>> + (abs @2)))))))
> >>>>>>>
> >>>>>>> I think this is overly complicated - an unnecessary conversion is
> >>>>>>> elided by
> >>>>>>> match so you should be able to write just
> >>>>>>>
> >>>>>>> (if (TYPE_UNSIGNED (type))
> >>>>>>> (absu (convert:stype @2))
> >>>>>>> (abs (convert:stype @2)))
> >>>>>>
> >>>>>> We also have to handle float type with convert:stype. Do you mean:
> >>>>>> (for cmp (ge gt)
> >>>>>> (simplify
> >>>>>> (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
> >>>>>> (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
> >>>>>> /* Support SEXT of @0 only. */
> >>>>>> && !TYPE_UNSIGNED (TREE_TYPE (@1))
> >>>>>> && element_precision (@1)
> >>>>>> <= element_precision (@0)
> >>>>>> && bitwise_equal_p (@1, @2))
> >>>>>> (with {
> >>>>>> tree stype = INTEGRAL_TYPE_P (type) ?
> >>>>>> signed_type_for (TREE_TYPE (@2)) : TREE_TYPE (@2);
> >>>>>> }
> >>>>>> /* Handle nop convert first to prevent additional
> >>>>>> convertion. */
> >>>>>> (if (INTEGRAL_TYPE_P (type)
> >>>>>> && element_precision (TREE_TYPE (@1))
> >>>>>> == element_precision (TREE_TYPE (@0)))
> >>>>>> (if (TYPE_UNSIGNED (type))
> >>>>>> (absu:type @0)
> >>>>>> (abs @0))
> >>>>>> (if (TYPE_UNSIGNED (TREE_TYPE (@2)))
> >>>>>> (absu (convert:stype @2))
> >>>>>> (abs (convert:stype @2)))))))
> >>>>>
> >>>>> I was thinking
> >>>>>
> >>>>> (for cmp (ge gt)
> >>>>> (simplify
> >>>>> (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
> >>>>> (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
> >>>>> /* Support SEXT of @0 only. */
> >>>>> && !TYPE_UNSIGNED (TREE_TYPE (@1))
> >>>>> && element_precision (@1)
> >>>>> <= element_precision (@0)
> >>>>> && bitwise_equal_p (@1, @2))
> >>>>> (if (TYPE_UNSIGNED (TREE_TYPE (@2)))
> >>>>> (with {
> >>>>> tree stype = signed_type_for (TREE_TYPE (@2));
> >>>>> }
> >>>>> (absu (convert:stype @2)))
> >>>>> (abs @2))))))
> >>>>>
> >>>>> if @2 is unsigned then 'type' is unsigned as well.
> >>>>
> >>>> Thanks. This works but this would require changing this:
> >>>> gcc.dg/tree-ssa/phi-opt-37.c: Check phiopt2 dump as shown in
> >>>> 0001-abshalffloat-simple.patch.
> >>>> If we do that optimisation Andrew suggested, it would be
> >>>> 0001-abshalffloat-handle_nop_convert.patch. This looks more complicated
> >>>> though.
> >>>
> >>> Meh, this is getting so tedious. Can you split out the (type)A >=/> 0
> >>> ? A : -A same as abs (A) part please?
> >>> We seem to have agreed on a version for that.
> >>
> >> Thanks. Attached patch does just this.
> >>
> >>
> >>>
> >>> I don't understand what the issue is with the other, and I'm tired to
> >>> investigate myself so please
> >>> explain to me.
> >>
> >> For gcc.dg/tree-ssa/phi-opt-37.c
> >> We have:
> >> <bb 2> :
> >> y_3 = (int) x_2(D);
> >> if (y_3 < 0)
> >> goto <bb 3>; [INV]
> >> else
> >> goto <bb 4>; [INV]
> >> <bb 3> :
> >> x_4 = -x_2(D);
> >> <bb 4> :
> >> # x_1 = PHI <x_2(D)(2), x_4(3)>
> >> return x_1;
> >>
> >>
> >> phiopt1 dump now shows (additional convert):
> >>
> >> phiopt match-simplify back:
> >> _5 = (signed int) x_2(D);
> >> _7 = ABSU_EXPR <_5>;
> >> result: _7
> >> rejected because early
> >>
> >> This is because there is a heuristics in phiopt that does not allow two
> >> inns here to enable certain optimisations. This however would get
> >> optimised later.
> >> May be this is ok as this would get optimised
> >
> > I see, so the point is to not "un-CSE" in the replacement if (convert?@0
> > @1) is
> > equal to (convert:stype @2). I think the same applies to the first pattern
> > and we should handle it consistently. In your previous -simple patch you
> > have
> >
> > + (for cmp (le lt)
> > + (simplify
> > + (cnd (cmp (convert?@0 @1) zerop) @2 (negate @2))
> > + (if (!HONOR_SIGNED_ZEROS (TREE_TYPE (@1))
> > + /* Support SEXT of @0 only. */
> > + && !TYPE_UNSIGNED (TREE_TYPE (@1))
> > + && element_precision (@1)
> > + <= element_precision (@0)
> > + && bitwise_equal_p (@1, @2))
> > + (if ((ANY_INTEGRAL_TYPE_P (TREE_TYPE (@1))
> > + && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
> >
> > I believe you want to check @2s type here since we use abs/absu on that.
> >
> > + || TYPE_UNSIGNED (TREE_TYPE(@2)))
> > + (with {
> > + tree stype = signed_type_for (TREE_TYPE (@2));
> > + tree utype = unsigned_type_for (TREE_TYPE (@1));
> > + }
> > + (convert (negate (absu:utype (convert:stype @2)))))
> >
> > instead of this do
> >
> > (if (types_match (@0, stype))
> > (convert (negate (absu:utype @0)))
> > (convert (negate (absu:utype (convert:stype @2))))
> >
> > + (convert (negate (abs @2))))))
> >
> > and adjust the other pattern similarly (we also want to avoid un-CSE there).
>
> Thanks for the review and suggestions. Please find the revised patch.
OK.
Thanks,
Richard.
> Thanks,
> Kugan
>
>
> >
> > Thanks and sorry for the very slow process.
> >
> > Richard.
> >
> >
> >> Bootstrapped and regression tests on aarch64-linux-gnu. Is this OK?
> >>
> >> Thanks,
> >> Kugan
> >>
> >>
> >>>
> >>> Richard.
> >>>
> >>>> Bootstrapped and regression tested both versions.
> >>>>
> >>>> Thanks,
> >>>> Kugan
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>
> >>>>>>>
> >>>>>>> btw, you can write element_precision (@1) directly, the function
> >>>>>>> handles
> >>>>>>> non-type arguments as well.
> >>>>>>
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Kugan
> >>>>>>>
> >>>>>>> Richard.
> >>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>> Kugan
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> Andrew Pinski
>
>
