diff mbox series

match: Check else value compatibility [PR123268].

Message ID DFIBXMI8XSJR.Y0N374TY7AHZ@gmail.com
State New
Headers
Series match: Check else value compatibility [PR123268]. |

Checks

Context Check Description
rivoscibot/toolchain-ci-rivos-lint warning Lint failed
rivoscibot/toolchain-ci-rivos-apply-patch success Patch applied
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-test fail Testing failed

Commit Message

Robin Dapp Jan. 7, 2026, 11:44 a.m. UTC 
  Hi,

In PR123268 we transform x * { 0 or 1, 0 or 1, ... } into
x & { 0 or -1, 0 or -1, ...} which is a match.pd pattern.

We start out with
  vect_b_28.48_48 = .COND_LEN_MUL ({ -1, -1, -1, -1 }, a, b, { 0.0, 0.0, 0.0, 0.0 }, 4, 0);
and simplify to

_67 = .COND_LEN_AND ({ -1, -1, -1, -1 }, _60, { 4294967295, 0, 0, 0 }, { 0.0, 0.0, 0.0, 0.0 }, 4, 0);

where the operation type is int but the else value (that we just copied
over from the original op) is still float.

In order to catch that, this patch checks if the operation type and the
else value type are compatible while reassembling the conditional
operation.

Bootstrapped and regtested on x86, power10, and aarch64.
Regtested on riscv64.

Regards
 Robin


gcc/ChangeLog:

	* gimple-match-exports.cc (convert_conditional_op): Check if
	orig_op->type and type of else value match.
---
 gcc/gimple-match-exports.cc | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Robin Dapp Jan. 7, 2026, 1:19 p.m. UTC | #1 
> +  if (!types_compatible_p (orig_op->type, TREE_TYPE (else_value)))
> +    return false;

Actually, I think we don't even want to accept nop-conversions here.
Just type equality should be acceptable.  I'll test that.
Richard Biener Jan. 7, 2026, 2:21 p.m. UTC | #2 
On Wed, Jan 7, 2026 at 2:23 PM Robin Dapp <rdapp.gcc@gmail.com> wrote:
>
> > +  if (!types_compatible_p (orig_op->type, TREE_TYPE (else_value)))
> > +    return false;
>
> Actually, I think we don't even want to accept nop-conversions here.
> Just type equality should be acceptable.  I'll test that.

But that's what types_compatible_p checks.  What I'd change is to only
verify this if else_value, not when that was determined by the
preferred else value hook.

Richard.

> --
> Regards
>  Robin
>
Robin Dapp Jan. 8, 2026, 1:40 p.m. UTC | #3 
> But that's what types_compatible_p checks.  What I'd change is to only
> verify this if else_value, not when that was determined by the
> preferred else value hook.


The below has been regtested and bootstrapped on x86, power10, aarch64.  
Regtested on riscv64.

Regards
 Robin


[PATCH v2] match: Check else value compatibility [PR123268].

In PR123268 we transform x * { 0 or 1, 0 or 1, ... } into
x & { 0 or -1, 0 or -1, ...} which is a match.pd pattern.

We start out with
  vect_b_28.48_48 = .COND_LEN_MUL ({ -1, -1, -1, -1 }, a, b, { 0.0, 0.0, 0.0, 0.0 }, 4, 0);
and simplify to

_67 = .COND_LEN_AND ({ -1, -1, -1, -1 }, _60, { 4294967295, 0, 0, 0 }, { 0.0, 0.0, 0.0, 0.0 }, 4, 0);

where the operation type is int but the else value (that we just copied
over from the original op) is still float.

In order to catch that, this patch checks if the operation type and the
else value type are compatible while reassembling the conditional
operation.

gcc/ChangeLog:

	* gimple-match-exports.cc (convert_conditional_op): Check if
	orig_op->type and type of else value match.
---
 gcc/gimple-match-exports.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gcc/gimple-match-exports.cc b/gcc/gimple-match-exports.cc
index 555e8a967a0..cdd9ae552e2 100644
--- a/gcc/gimple-match-exports.cc
+++ b/gcc/gimple-match-exports.cc
@@ -184,6 +184,13 @@ convert_conditional_op (gimple_match_op *orig_op,
   for (unsigned int i = 0; i < num_ops; ++i)
     new_op->ops[i + 1] = orig_op->ops[i];
   tree else_value = orig_op->cond.else_value;
+  /* Some patterns convert operand types, e.g. from float to int.
+     If we had a real else value before (e.g. float) it won't match
+     the type.  Verify that here.  */
+  if (else_value
+      && !types_compatible_p (orig_op->type, TREE_TYPE (else_value)))
+    return false;
+
   if (!else_value)
     else_value = targetm.preferred_else_value (ifn, orig_op->type,
 					       num_ops, orig_op->ops);
Richard Biener Jan. 8, 2026, 1:58 p.m. UTC | #4 
On Thu, Jan 8, 2026 at 2:40 PM Robin Dapp <rdapp.gcc@gmail.com> wrote:
>
> > But that's what types_compatible_p checks.  What I'd change is to only
> > verify this if else_value, not when that was determined by the
> > preferred else value hook.
>
>
> The below has been regtested and bootstrapped on x86, power10, aarch64.
> Regtested on riscv64.

LGTM.

Richard.

> Regards
>  Robin
>
>
> [PATCH v2] match: Check else value compatibility [PR123268].
>
> In PR123268 we transform x * { 0 or 1, 0 or 1, ... } into
> x & { 0 or -1, 0 or -1, ...} which is a match.pd pattern.
>
> We start out with
>   vect_b_28.48_48 = .COND_LEN_MUL ({ -1, -1, -1, -1 }, a, b, { 0.0, 0.0, 0.0, 0.0 }, 4, 0);
> and simplify to
>
> _67 = .COND_LEN_AND ({ -1, -1, -1, -1 }, _60, { 4294967295, 0, 0, 0 }, { 0.0, 0.0, 0.0, 0.0 }, 4, 0);
>
> where the operation type is int but the else value (that we just copied
> over from the original op) is still float.
>
> In order to catch that, this patch checks if the operation type and the
> else value type are compatible while reassembling the conditional
> operation.
>
> gcc/ChangeLog:
>
>         * gimple-match-exports.cc (convert_conditional_op): Check if
>         orig_op->type and type of else value match.
> ---
>  gcc/gimple-match-exports.cc | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/gcc/gimple-match-exports.cc b/gcc/gimple-match-exports.cc
> index 555e8a967a0..cdd9ae552e2 100644
> --- a/gcc/gimple-match-exports.cc
> +++ b/gcc/gimple-match-exports.cc
> @@ -184,6 +184,13 @@ convert_conditional_op (gimple_match_op *orig_op,
>    for (unsigned int i = 0; i < num_ops; ++i)
>      new_op->ops[i + 1] = orig_op->ops[i];
>    tree else_value = orig_op->cond.else_value;
> +  /* Some patterns convert operand types, e.g. from float to int.
> +     If we had a real else value before (e.g. float) it won't match
> +     the type.  Verify that here.  */
> +  if (else_value
> +      && !types_compatible_p (orig_op->type, TREE_TYPE (else_value)))
> +    return false;
> +
>    if (!else_value)
>      else_value = targetm.preferred_else_value (ifn, orig_op->type,
>                                                num_ops, orig_op->ops);
> --
> 2.52.0
>
Jeffrey Law Jan. 8, 2026, 3:38 p.m. UTC | #5 
On 1/8/2026 6:58 AM, Richard Biener wrote:
> On Thu, Jan 8, 2026 at 2:40 PM Robin Dapp <rdapp.gcc@gmail.com> wrote:
>>> But that's what types_compatible_p checks.  What I'd change is to only
>>> verify this if else_value, not when that was determined by the
>>> preferred else value hook.
>>
>> The below has been regtested and bootstrapped on x86, power10, aarch64.
>> Regtested on riscv64.
> LGTM.
I've pushed this to the trunk.  I'll take care of BZ momentarily.
jeff
diff mbox series

Patch

diff --git a/gcc/gimple-match-exports.cc b/gcc/gimple-match-exports.cc
index 555e8a967a0..9f08f86dabe 100644
--- a/gcc/gimple-match-exports.cc
+++ b/gcc/gimple-match-exports.cc
@@ -187,6 +187,13 @@  convert_conditional_op (gimple_match_op *orig_op,
   if (!else_value)
     else_value = targetm.preferred_else_value (ifn, orig_op->type,
 					       num_ops, orig_op->ops);
+
+  /* Some patterns convert operand types, e.g. from float to int.
+     If we had a real else value before (e.g. float) it won't match
+     the type.  Check that here.  */
+  if (!types_compatible_p (orig_op->type, TREE_TYPE (else_value)))
+    return false;
+
   new_op->ops[num_ops + 1] = else_value;
   if (orig_op->cond.len)
     {