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