[PATCH 05/20] tcg/optimize: Build and use z_bits and o_bits in fold_eqv

Richard Henderson posted 20 patches 7 months, 2 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>
There is a newer version of this series
[PATCH 05/20] tcg/optimize: Build and use z_bits and o_bits in fold_eqv
Posted by Richard Henderson 7 months, 2 weeks ago
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/optimize.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index faee3e8580..08d15e5395 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -1917,7 +1917,7 @@ static bool fold_dup2(OptContext *ctx, TCGOp *op)
 
 static bool fold_eqv(OptContext *ctx, TCGOp *op)
 {
-    uint64_t s_mask;
+    uint64_t z_mask, o_mask, s_mask;
     TempOptInfo *t1, *t2;
 
     if (fold_const2_commutative(ctx, op) ||
@@ -1947,8 +1947,12 @@ static bool fold_eqv(OptContext *ctx, TCGOp *op)
     }
 
     t1 = arg_info(op->args[1]);
+
+    z_mask = (t1->z_mask | ~t2->o_mask) & (t2->z_mask | ~t1->o_mask);
+    o_mask = ~(t1->z_mask | t2->z_mask) | (t1->o_mask & t2->o_mask);
     s_mask = t1->s_mask & t2->s_mask;
-    return fold_masks_s(ctx, op, s_mask);
+
+    return fold_masks_zos(ctx, op, z_mask, o_mask, s_mask);
 }
 
 static bool fold_extract(OptContext *ctx, TCGOp *op)
-- 
2.43.0
Re: [PATCH 05/20] tcg/optimize: Build and use z_bits and o_bits in fold_eqv
Posted by Pierrick Bouvier 5 months, 3 weeks ago
On 5/5/25 1:27 PM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/optimize.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index faee3e8580..08d15e5395 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -1917,7 +1917,7 @@ static bool fold_dup2(OptContext *ctx, TCGOp *op)
>   
>   static bool fold_eqv(OptContext *ctx, TCGOp *op)
>   {
> -    uint64_t s_mask;
> +    uint64_t z_mask, o_mask, s_mask;
>       TempOptInfo *t1, *t2;
>   
>       if (fold_const2_commutative(ctx, op) ||
> @@ -1947,8 +1947,12 @@ static bool fold_eqv(OptContext *ctx, TCGOp *op)
>       }
>   
>       t1 = arg_info(op->args[1]);
> +
> +    z_mask = (t1->z_mask | ~t2->o_mask) & (t2->z_mask | ~t1->o_mask);
> +    o_mask = ~(t1->z_mask | t2->z_mask) | (t1->o_mask & t2->o_mask);
>       s_mask = t1->s_mask & t2->s_mask;

Even after writing the truth table for eqv(t1, t2) = ~(t1 ^ t2), I'm not 
sure to understand directly how z_mask and o_mask are derived.

In this case, we have:
t1 | t2 | ~(t1 ^ t2)
0  | 0  | 1
0  | 1  | 0
1  | 0  | 0
1  | 1  | 1

In this commit, and in the series, it's confusing for me to have mask 
values set as 0 for 0, and 1 for 1. When mixing that with bitwise 
operations, it starts to get hard to follow, always having to remember 
if you deal with 0 or 1.

It could really help to have simple helpers for (known) zeroes(v) and 
ones(v). I feel as well some comments would be removed because it would 
become explicit what we are dealing with.

let:
   zeroes(v) = ~v->z_mask
   ones(v) = v->o_mask

res_zeroes = zeroes(t1) & ones(t2) | ones(t1) & zeroes(t2);
z_mask = ~res_zeroes;

which gives:
z_mask = ~zeroes
        = ~((~t1->z & t2->o) | (t1->o & ~t2->z))
        = ~(~t1->z & t2->o) & ~(t1->o & ~t2->z)
        = (t1->z | ~t2->o) | (~t1->o | t2->z)
which is the code we have here.

> -    return fold_masks_s(ctx, op, s_mask);
> +
> +    return fold_masks_zos(ctx, op, z_mask, o_mask, s_mask);
>   }
>   
>   static bool fold_extract(OptContext *ctx, TCGOp *op)

I'm not necessarily forcing a change, but I don't see myself rewriting 
truth tables and developing expressions on paper for all operations to 
review they are correct.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>