[PATCH vkd3d v7 5/6] vkd3d-shader/hlsl: Handle conditionals in copy propagation.

[Zebediah Figura]

On 12/1/21 10:12, Matteo Bruni wrote:
> On Thu, Nov 18, 2021 at 10:01 AM Giovanni Mascellani
> <gmascellani at codeweavers.com> wrote:
>>
>> Hi,
>>
>> On 18/11/21 05:18, Zebediah Figura (she/her) wrote:
>>>>> +    if (!hlsl_array_reserve(ctx, (void**)&state->variables,
>>>>> &state->capacity, state->depth + 2, sizeof(*state->variables)))
>>>>
>>>> "state->depth + 2" looks wrong; is it?
>>>
>>> Wait, never mind, I see why this looks wrong. The fact that "depth" is
>>> 1-biased seems confusing to me, though...
>>
>> I agree it is a bit strange, but I think it's correct too.
>>
>> As whether to store the recursion depth or the number of stack levels
>> (which is depth + 1), I think it's a matter of tastes. If you want "+ 1"
>> here, you'll have a lot of "- 1" in other places (mostly _get_variable
>> and _create_variable).
> 
> True, although one could argue that having a bunch of "- 1" in those
> helper functions is proper enough.
> 
> In this particular case though, when I see:
> 
> struct rb_entry *entry = rb_get(&state->variables[state->depth], var);
> 
> I immediately think that the array access is missing a "- 1", as that
> is the usual (for d3d code at least) pattern of accessing the last
> element of an array with "count" elements.
> 

Yeah, I concur. Accessing the "last" element via "[count - 1]" is 
idiomatic enough that omitting it looks wrong.