[PATCH 7/7] d3dcompiler: Store the "array" field in struct hlsl_deref as a struct hlsl_deref pointer.

[Matteo Bruni]

On Tue, Apr 7, 2020 at 3:52 AM Zebediah Figura <zfigura at codeweavers.com> wrote:
>
> I gave it some examination and though and I decided I really don't like
> the current solution, for reasons which I mostly go into detail about in
> the attached patch 0001. I came up with two alternative approaches, and
> I'm not really sure which one I prefer:
>
> * patch 0001 just separates source and destination derefs, forcing the
> latter to be a chain of derefs ending in a var while the former can end
> in an arbitrary node, which has the advantage of generating less HLSL
> (not that it particularly matters);
>
> * patches 0100-0102 synthesizes a variable every time we encounter a
> subscript/index of anything other than a variable, thus implicitly
> forcing all derefs to be a chain of hlsl_deref ending in a var—which has
> the advantage of letting weird LHS constructions work "for free" (not
> that it particularly matters either).
>
> As an aside, I think that if a structure splitting pass does happen, we
> would need for arbitrary instructions to be able to hold either a
> hlsl_deref or an hlsl_ir_node for any source fields. I'm not actually
> sure that makes either approach better, but it seems at least worth
> thinking about.
>
> Any opinions?

Starting from the last point, yeah, I think we need to preserve the
possibility of having a deref (rather than a whole var) as the
"ultimate" node in any instruction. I don't think it matters for the
issue at hand but it's something to keep in mind.

With that out of the way... I do agree in general about the
awkwardness of the current solution. I'd make it more explicit: it's
fundamentally a matter of distinguishing lvalue derefs vs rvalue
derefs (the former isn't really a "read", in terms of semantics and
also e.g. liveness). If you mark lvalue derefs in some way you should
be able to fix liveness properly, and probably avoid some other
headaches down the line. They don't have to be two separate
hlsl_ir_node_type necessarily, although that's an option.

Your patch 0001 basically does that and enforces that the LHS of an
assignment is an lvalue, if maybe not as explicitly. Also, since in
"old" HLSL there are no pointers, there shouldn't be many other
interesting cases for lvalues aside from assignments, so it probably
does the job.
Unrelated note: I agree with the comment in the patch about leaving
cleanup to a following pass. I found it's usually a lot easier to
leave "tidying up" to separate optimization passes (even trivial ones)
rather than trying to generate "perfect" code right away. Exceptions
apply, but that seems to be a generally good idea.

Patches 0100-0102 also certainly work. If you go with something like
patch 0001 you won't need these now, but I suspect they will come in
handy anyway, eventually.