diff mbox series

[03/46] tcg/optimize: Add fold_masks_zsa, fold_masks_zs, fold_masks_z

Message ID 20241210152401.1823648-4-richard.henderson@linaro.org
State New
Headers show
Series tcg: Remove in-flight mask data from OptContext | expand

Commit Message

Richard Henderson Dec. 10, 2024, 3:23 p.m. UTC 
Add additional routines to pass masks directly, rather than
storing them into OptContext.  To be used in upcoming patches.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/optimize.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

Comments

Pierrick Bouvier Dec. 17, 2024, 8:03 p.m. UTC | #1 
On 12/10/24 07:23, Richard Henderson wrote:
> Add additional routines to pass masks directly, rather than
> storing them into OptContext.  To be used in upcoming patches.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/optimize.c | 24 ++++++++++++++++++++----
>   1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index 1a9e3258e3..6644d24da6 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -1045,11 +1045,9 @@ static bool fold_const2_commutative(OptContext *ctx, TCGOp *op)
>       return fold_const2(ctx, op);
>   }
>   
> -static bool fold_masks(OptContext *ctx, TCGOp *op)
> +static bool fold_masks_zsa(OptContext *ctx, TCGOp *op, uint64_t z_mask,
> +                           uint64_t s_mask, uint64_t a_mask)
>   {
> -    uint64_t a_mask = ctx->a_mask;
> -    uint64_t z_mask = ctx->z_mask;
> -    uint64_t s_mask = ctx->s_mask;
>       const TCGOpDef *def = &tcg_op_defs[op->opc];
>       TCGTemp *ts;
>   
> @@ -1083,6 +1081,24 @@ static bool fold_masks(OptContext *ctx, TCGOp *op)
>       return true;
>   }
>   
> +__attribute__((unused))
> +static bool fold_masks_zs(OptContext *ctx, TCGOp *op,
> +                          uint64_t z_mask, uint64_t s_mask)
> +{
> +    return fold_masks_zsa(ctx, op, z_mask, s_mask, -1);
> +}
> +
> +__attribute__((unused))
> +static bool fold_masks_z(OptContext *ctx, TCGOp *op, uint64_t z_mask)
> +{
> +    return fold_masks_zsa(ctx, op, z_mask, smask_from_zmask(z_mask), -1);
> +}
> +
> +static bool fold_masks(OptContext *ctx, TCGOp *op)
> +{
> +    return fold_masks_zsa(ctx, op, ctx->z_mask, ctx->s_mask, ctx->a_mask);
> +}
> +
>   /*
>    * Convert @op to NOT, if NOT is supported by the host.
>    * Return true f the conversion is successful, which will still

I see the direction, but why not simply use a structure for this?

If I understand correctly, we'll only pass the masks to callees, so it's 
easy to pass the pointer down (without any heap allocation needed), and 
we can have associated builder functions to create the struct only with 
a limited set of masks, or directly from an existing "ctx".
Richard Henderson Dec. 18, 2024, 3:20 a.m. UTC | #2 
On 12/17/24 14:03, Pierrick Bouvier wrote:
>> +__attribute__((unused))
>> +static bool fold_masks_zs(OptContext *ctx, TCGOp *op,
>> +                          uint64_t z_mask, uint64_t s_mask)
>> +{
>> +    return fold_masks_zsa(ctx, op, z_mask, s_mask, -1);
>> +}
>> +
>> +__attribute__((unused))
>> +static bool fold_masks_z(OptContext *ctx, TCGOp *op, uint64_t z_mask)
>> +{
>> +    return fold_masks_zsa(ctx, op, z_mask, smask_from_zmask(z_mask), -1);
>> +}
>> +
>> +static bool fold_masks(OptContext *ctx, TCGOp *op)
>> +{
>> +    return fold_masks_zsa(ctx, op, ctx->z_mask, ctx->s_mask, ctx->a_mask);
>> +}
>> +
>>   /*
>>    * Convert @op to NOT, if NOT is supported by the host.
>>    * Return true f the conversion is successful, which will still
> 
> I see the direction, but why not simply use a structure for this?
> 
> If I understand correctly, we'll only pass the masks to callees, so it's easy to pass the 
> pointer down (without any heap allocation needed), and we can have associated builder 
> functions to create the struct only with a limited set of masks, or directly from an 
> existing "ctx".

Why would we want to use a structure?  I'm confused by the question.


r~
Pierrick Bouvier Dec. 18, 2024, 7:11 p.m. UTC | #3 
On 12/17/24 19:20, Richard Henderson wrote:
> On 12/17/24 14:03, Pierrick Bouvier wrote:
>>> +__attribute__((unused))
>>> +static bool fold_masks_zs(OptContext *ctx, TCGOp *op,
>>> +                          uint64_t z_mask, uint64_t s_mask)
>>> +{
>>> +    return fold_masks_zsa(ctx, op, z_mask, s_mask, -1);
>>> +}
>>> +
>>> +__attribute__((unused))
>>> +static bool fold_masks_z(OptContext *ctx, TCGOp *op, uint64_t z_mask)
>>> +{
>>> +    return fold_masks_zsa(ctx, op, z_mask, smask_from_zmask(z_mask), -1);
>>> +}
>>> +
>>> +static bool fold_masks(OptContext *ctx, TCGOp *op)
>>> +{
>>> +    return fold_masks_zsa(ctx, op, ctx->z_mask, ctx->s_mask, ctx->a_mask);
>>> +}
>>> +
>>>    /*
>>>     * Convert @op to NOT, if NOT is supported by the host.
>>>     * Return true f the conversion is successful, which will still
>>
>> I see the direction, but why not simply use a structure for this?
>>
>> If I understand correctly, we'll only pass the masks to callees, so it's easy to pass the
>> pointer down (without any heap allocation needed), and we can have associated builder
>> functions to create the struct only with a limited set of masks, or directly from an
>> existing "ctx".
> 
> Why would we want to use a structure?  I'm confused by the question.
>

When I started reading the series, I thought we would keep the flags in 
ctx structure, but after finishing it, I understood better what was the 
intention.

That said, it's just a matter of preference, between having individual 
variables or having a struct containing them, with functions to operate 
on it.

> 
> r~
diff mbox series

Patch

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 1a9e3258e3..6644d24da6 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -1045,11 +1045,9 @@  static bool fold_const2_commutative(OptContext *ctx, TCGOp *op)
     return fold_const2(ctx, op);
 }
 
-static bool fold_masks(OptContext *ctx, TCGOp *op)
+static bool fold_masks_zsa(OptContext *ctx, TCGOp *op, uint64_t z_mask,
+                           uint64_t s_mask, uint64_t a_mask)
 {
-    uint64_t a_mask = ctx->a_mask;
-    uint64_t z_mask = ctx->z_mask;
-    uint64_t s_mask = ctx->s_mask;
     const TCGOpDef *def = &tcg_op_defs[op->opc];
     TCGTemp *ts;
 
@@ -1083,6 +1081,24 @@  static bool fold_masks(OptContext *ctx, TCGOp *op)
     return true;
 }
 
+__attribute__((unused))
+static bool fold_masks_zs(OptContext *ctx, TCGOp *op,
+                          uint64_t z_mask, uint64_t s_mask)
+{
+    return fold_masks_zsa(ctx, op, z_mask, s_mask, -1);
+}
+
+__attribute__((unused))
+static bool fold_masks_z(OptContext *ctx, TCGOp *op, uint64_t z_mask)
+{
+    return fold_masks_zsa(ctx, op, z_mask, smask_from_zmask(z_mask), -1);
+}
+
+static bool fold_masks(OptContext *ctx, TCGOp *op)
+{
+    return fold_masks_zsa(ctx, op, ctx->z_mask, ctx->s_mask, ctx->a_mask);
+}
+
 /*
  * Convert @op to NOT, if NOT is supported by the host.
  * Return true f the conversion is successful, which will still