Re: [PATCH vkd3d v3 13/17] vkd3d-shader/hlsl: Replace register offsets with index paths in resource loads initialization.

[Francisco Casas]

Ah, I finally see, the

hlsl_src_from_node(&deref->offset, other->offset.node);

indeed is a noop. I will fix it.


On 25-07-22 04:11, Giovanni Mascellani wrote:
> Hi,
>
> Il 22/07/22 20:36, Francisco Casas ha scritto:
> > > > diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
> > > > index 2d5fe40a..a4d594b7 100644
> > > > --- a/libs/vkd3d-shader/hlsl.c
> > > > +++ b/libs/vkd3d-shader/hlsl.c
> > > > @@ -862,6 +862,28 @@ static bool type_is_single_reg(const struct 
> > > > hlsl_type *type)
> > > >       return type->type == HLSL_CLASS_SCALAR || type->type == 
> > > > HLSL_CLASS_VECTOR;
> > > >   }
> > > > +static bool deref_copy(struct hlsl_ctx *ctx, struct hlsl_deref 
> > > > *deref, struct hlsl_deref *other)
> > > > +{
> > > > +    unsigned int i;
> > > > +
> > > > +    memset(deref, 0, sizeof(*deref));
> > > > +
> > > > +    if (!other)
> > > > +        return true;
> > > > +
> > > > +    assert(!other->offset.node);
> > > > +
> > > > +    if (!init_deref(ctx, deref, other->var, other->path_len))
> > > > +        return false;
> > > > +
> > > > +    for (i = 0; i < deref->path_len; ++i)
> > > > +        hlsl_src_from_node(&deref->path[i], other->path[i].node);
> > > > +
> > > > +    hlsl_src_from_node(&deref->offset, other->offset.node);
> > >
> > > You just asserted that other->offset.node is NULL and *deref was 
> > > initialized to zero, so that seems to be a noop, doesn't it?
> >
> > Hmm, sorry, I am not sure I understand.
> > Initializing *deref to zero shouldn't affect
> >
> > assert(!other->offset.node);
> >
> > The idea of this assertion is to ensure that the "other" deref is in 
> > the index path format instead of the register offset format.
>
> My point is that, given that you just asserted that other->offset.node 
> is NULL, the effect of "hlsl_src_from_node(&deref->offset, 
> other->offset.node)" would be to set deref->offset.node to NULL, but 
> that's already ensured by the memset(). Or maybe I am getting confused 
> on something trivial?
>
> Giovanni.