Re: [PATCH v9 0/5] MR409: vkd3d-shader/dxil: Implement the DXIL BINOP instruction.

[Conor McCarthy (＠cmccarthy)]

On Thu Nov  9 04:30:28 2023 +0000, Henri Verbeet wrote:
> > > Sure. I think the main alternative would be to split the IR in two
> > > (or more) separate IRs though. I.e., you'd have a representation of the
> > > parsed TPF, then convert that to VSIR, and from there to SPIR-V.
> >
> > The alternative I was considering would rather be to have only one IR,
> > and use it for all shader language conversions, but not for
> > disassembling. And then, for each language for which we support
> > disassembling, have a dedicated disassembler (which probably doesn't
> > really need an IR: it can emit as it parses). If we supported assembling
> > too, we'd also have a dedicated assembler.
> > Well, we could debate at which point something gets to be called an
> > "IR", but essentially, we'd need to have some function that would e.g.
> > parse TPF bytecode, and return something that might be called a "struct
> > tpf_instruction". We'd then pass that to either the disassembler, or
> > something that translates it to vsir. The main point is that we wouldn't
> > be able to parse directly into vsir structures, unless we'd duplicate
> > the parsing code for the disassembler.
> > In my mind, assembling/disassembling and compiling (or transpiling, if
> > we want to look more modern!) are two different beasts. For compiling
> > it's useful to have an IR which is flexible and simple, but it doesn't
> > need to faithfully represent precisely all the features of any other
> > language. OTOH for assembling/disassembling you don't really care about
> > flexibility, but it's important to represent faithfully every detail of
> > the language.
> >
> > My feeling is that trying to shove all these features (simplicity,
> > flexibility, faithfullness to any language) on a single language is a
> > bit overconstraining. Write dedicated assemblers and disassemblers is
> > some additional work too, but I'm not sure the balance is in favor of
> > our solution.
> > I tend to think it is, but I could certainly imagine different people
> > striking different balances.
> > I suppose the good news is that it's relatively easy to have a go at
> > giving it a try. My guess is that we'd end up duplicating most of the
> > vsir structures, duplicate most of the disassembler, introduce extra
> > tpf->vsir and vsir->tpf passes, and then would be able to remove some
> > fields and enum elements from the vsir data structures. It's entirely
> > possible that I'm underestimating the benefits though. And sure, I can
> > see the appeal of having a more restricted/pure IR, having stronger
> > separation between d3dbc/tpf/dxil/spirv/etc.; the practical benefits
> > aren't quite as clear to me though.
> > > The disassembler would operate on TPF IR, as would certain lowering
> > > passes. That's certainly a valid choice, but I think it's important to
> > > point out that while it would make some thing easier, it would also make
> > > some things harder. The most obvious is perhaps that we'd need separate
> > > disassemblers for d3dbc, tpf, dxil, and vsir. Somewhat less obvious is
> > > perhaps that we may need to duplicate certain lowering passes between
> > > e.g. d3dbc and tpf, because we'd no longer be able to express them in
> > > vsir. It may also make it slightly harder to do something like HLSL IR
> > > -> vsir -> d3dbc, because we'd have to get rid of complex texturing
> > > instructions when converting HLSL IR to vsir, and then reintroduce them
> > > when converting vsir to d3dbc.
> >
> > If VSIR features are useful for translating between languages, then I
> > agree it's sensible to have then. The part I don't like is having
> > features only because some of the languages we support need to
> > faithfully represent all their features (e.g., having to keep operations
> > like NEG and ABS as register modifiers instead of as regular operators).
> > Note that NEG and ABS modifiers aren't only used by the disassembler
> > though; we also use these when writing output for the HLSL compiler. And
> > sure, there are other ways to handle that as well, but as far as I can
> > see they'd all be variants of "introduce struct tpf_instruction".
> > > We may want to tweak the vkd3d_shader_instruction_array data
> > > structure somewhat, but I don't think we'll need to do anything as
> > > drastic as converting the instruction array to a linked list; gap
> > > buffers tend to handle this kind of thing fairly well, and we may even
> > > be able to improve on that in specific instances.
> >
> > I don't know much about gap buffers, but after some reading on
> > Wikipedia I'm not convinced. It seems that a gap buffers makes sense
> > when you have a concept of a cursor that mostly moves locally, while our
> > passes usually scan the whole program each time. With a gap buffer you
> > would end up copying the whole program each time, and by that token you
> > could directly rewrite it in a new array each time.
> > I think that's a misunderstanding; the amount of data you'd move would
> > only be the difference between the previous insertion position and the
> > new insertion position. I.e., if you start with a sufficiently large gap
> > at the front of the buffer, and move through the instructions from first
> > to last, you'll move at most a single copy of the entire program in the
> > worst case, and likely a fair bit less in typical cases.
> > Also, random insertion with gap buffers seems to be comparable to arrays.
> > Random insertion should be rare though(!); the more typical case would
> > be to go through the instructions from first to last, or possibly from
> > last to first.
> > Following some link on Wikipedia, a
> > [rope](https://en.wikipedia.org/wiki/Rope_\(data_structure\)) might be a
> > better match for us, being essentially a compromise between an array and
> > a link-based structure.
> > Yeah, I'm aware of ropes. It doesn't seem obvious to me that they'd be
> > better suited for our purposes, but ultimately I think this is one of
> > those things where you start with the naive implementation, and then see
> > whether making it more complicated makes it better.
> > FWIW I have largely complete backend code for a low-level IR (without registers), which I wrote early on in SM6 development. It's a bit over half the size of the existing backend, but the lowering phase for TPF would certainly make up for that. I guess the only reason to make such a major change would be so it's simpler to add new backends.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/409#note_51640