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

[Zebediah Figura]

On 11/23/21 5:10 AM, Giovanni Mascellani wrote:
> Signed-off-by: Giovanni Mascellani <gmascellani at codeweavers.com>
> ---
> 
> 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...)
> 

I don't know that it's worth the trouble. Any given operation should be 
able to support vec1's as well as scalars. We have a helper for that, 
after all.

>> @@ -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.

I don't think there's any particular reason, no.