Re: [PATCH vkd3d 4/6] vkd3d-shader/hlsl: Lower narrowing casts to swizzles.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani 
---

On 23/11/21 02:45, Zebediah Figura wrote:
> +static bool lower_narrowing_casts(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
> +{
> - const struct hlsl_type *src_type, *dst_type;
> - struct hlsl_ir_expr *cast;
> -
> - if (instr->type != HLSL_IR_EXPR)
> -
>    return false;
> - cast = hlsl_ir_expr(instr);
> - src_type = cast->operands[0].node->data_type;
> - dst_type = cast->node.data_type;
> -
> - if (cast->op == HLSL_OP1_CAST
> -
>        && src_type->type <= HLSL_CLASS_VECTOR && dst_type->type <= HLSL_CLASS_VECTOR
> -
>        && dst_type->dimx < src_type->dimx)
> - {
> -
>    struct hlsl_ir_swizzle *swizzle;
> -
> -
>    if (!(swizzle = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(X, Y, Z, W), dst_type->dimx, &cast->node, &cast->node.loc)))
> -
>        return false;
> -
>    list_add_after(&cast->node.entry, &swizzle->node.entry);
> -
> -
>    cast->node.data_type = hlsl_get_vector_type(ctx, dst_type->base_type, src_type->dimx);
> -
>    replace_node(&cast->node, &swizzle->node);
> -
>    hlsl_src_remove(&swizzle->val);
> -
>    hlsl_src_from_node(&swizzle->val, &cast->node);
> -
>    return true;
> - }
> -
> - return false;
>
> +}

The same comments of 3/6 apply here.

While reviewing this patch it occurred to me that both this and 3/6 can 
change the type of a node, by turning a scalar into a vector of 
dimension 1. I guess that by this point the code is sufficiently 
low-level that we don't care any more, but since this a bit vague, I'd 
propose to identify the point where all scalars in the IR can be 
converted to vectors of dimension 1, and from that point on scalars are 
not used any more. What do you think of that? (I like strongly types 
data...)
> @@ -1706,6 +1737,7 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun
>           progress |= transform_ir(ctx, split_struct_copies, body, NULL);
>       }
>       while (progress);
> - transform_ir(ctx, lower_narrowing_casts, body, NULL);
>  do
>  {
>      progress = transform_ir(ctx, fold_constants, body, NULL);

Is there a specific reason for how you are ordering this pass (and 
3/6?). Not that I have any problem with it, it is just that 4/6 seems 
similar to 3/6, but you put them in two different places.

For the record, I have another pass in my development patches that 
generalizes both 3/6 and 4/6 and also works with matrices. It will take 
some time to have it accepted, if it will ever be, so it should not be a 
reason to delay these two patches.

Thanks, Giovanni.