From: Giovanni Mascellani <gmascellani@codeweavers.com>
Subject: Re: [PATCH vkd3d v5 4/6] vkd3d-shader/hlsl: Perform a copy propagation pass.
Message-Id: <c520be89-d053-4c5e-fa13-bce37b88217c@codeweavers.com>
Date: Tue, 16 Nov 2021 10:04:56 +0100
In-Reply-To: <20211115142716.1529517-4-gmascellani@codeweavers.com>
References: <20211115142716.1529517-1-gmascellani@codeweavers.com> <20211115142716.1529517-4-gmascellani@codeweavers.com>

Hi,

On 15/11/21 15:27, Giovanni Mascellani wrote:
> +static void copy_propagation_set_value(struct copy_propagation_variable *variable, unsigned int offset,
> +        unsigned char writemask, struct hlsl_ir_node *node)
> +{
> +    static const char swizzle_letters[] = {'x', 'y', 'z', 'w'};
> +
> +    unsigned int i;
> +
> +    for (i = 0; i < 4; ++i)
> +    {
> +        if (writemask & (1u << i))
> +        {
> +            TRACE("Variable %s[%d] is written by instruction %p.%c.\n",
> +                    variable->var->name, offset + i, node, swizzle_letters[i]);
> +            variable->values[offset + i].node = node;
> +            variable->values[offset + i].component = i;
> +        }
> +    }
> +}

After playing with the code a little more, I don't think that's correct 
any more. What we call the "writemask" in struct hlsl_ir_store seems 
more a "write-swizzle", right? The correct code would be:

j = 0;
for (i = 0; i < 4; ++i)
{
     if (writemask & (1u << i))
     {
         ...
         variable->values[offset + i].component = j++;
     }
}

In other words, if we are storing with a writemask .y, we mean that the 
first register of the source goes in the second register of the 
destination, not that the second register of the source goes in the 
second register of the destination. Otherwise the code generated by the 
constructor float4(1.0, 2.0, 3.0, 4.0) would be wrong:

trace:hlsl_dump_function:    2:      float | 1.00000000e+00
trace:hlsl_dump_function:    3:      float | 2.00000000e+00
trace:hlsl_dump_function:    4:      float | 3.00000000e+00
trace:hlsl_dump_function:    5:      float | 4.00000000e+00
trace:hlsl_dump_function:    6:            | = (<constructor-0>.x @2)
trace:hlsl_dump_function:    7:            | = (<constructor-0>.y @3)
trace:hlsl_dump_function:    8:            | = (<constructor-0>.z @4)
trace:hlsl_dump_function:    9:            | = (<constructor-0>.w @5)
trace:hlsl_dump_function:   10:     float4 | <constructor-0>

Does any of this make sense?

Giovanni.