On Mon, Oct 21, 2024 at 4:23 PM Akram Ahmad <akram.ah...@arm.com> wrote:
>
> Case 7 of unsigned scalar saturating addition defines
> SAT_ADD = X <= (X + Y) ? (X + Y) : -1. This is the same as
> SAT_ADD = Y <= (X + Y) ? (X + Y) : -1 due to usadd_left_part_1
> being commutative.
>
> The pattern for case 7 currently does not accept the alternative
> where Y is used in the condition. Therefore, this commit adds the
> commutative property to this case which causes more valid cases of
> unsigned saturating arithmetic to be recognised.
>
> Before:
> <bb 2>
> _1 = BIT_FIELD_REF <b_3(D), 8, 0>;
> sum_5 = _1 + a_4(D);
> if (a_4(D) <= sum_5)
> goto <bb 4>; [INV]
> else
> goto <bb 3>; [INV]
>
> <bb 3> :
>
> <bb 4> :
> _2 = PHI <255(3), sum_5(2)>
> return _2;
>
> After:
> <bb 2> [local count: 1073741824]:
> _1 = BIT_FIELD_REF <b_3(D), 8, 0>;
> _2 = .SAT_ADD (_1, a_4(D)); [tail call]
> return _2;
>
> This passes the aarch64-none-linux-gnu regression tests with no new
> failures.
I think this boils down to
(match (usadd_left_part_1 @0 @1)
(plus:c @0 @1)
^^^ the :c here doesn't make much sense
(if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type)
&& types_match (type, @0, @1))))
but ...
> gcc/ChangeLog:
>
> * match.pd: Modify existing case for SAT_ADD.
> ---
> gcc/match.pd | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 4fc5efa6247..a77fca92181 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3166,7 +3166,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> /* Unsigned saturation add, case 7 (branch with le):
> SAT_ADD = x <= (X + Y) ? (X + Y) : -1. */
> (match (unsigned_integer_sat_add @0 @1)
> - (cond^ (le @0 (usadd_left_part_1@2 @0 @1)) @2 integer_minus_onep))
> + (cond^ (le @0 (usadd_left_part_1:c@2 @0 @1)) @2 integer_minus_onep))
... this is falsely accepted by genmatch I think. You should be using
:C (capital C,
"I know what I'm doing here") here since genmatch has no way to verify
usadd_left_part_1
is commutative or not.
Please also add a testcase.
Can you check whether removing the :c from the (plus in
usadd_left_part_1 keeps things
working?
Thanks,
Richard.
>
> /* Unsigned saturation add, case 8 (branch with gt):
> SAT_ADD = x > (X + Y) ? -1 : (X + Y). */
> --
> 2.34.1
>
