Skip to content
Snippets Groups Projects

Consolidate the TPF and d3dbc handling code.

Merged Henri Verbeet requested to merge hverbeet/vkd3d:for_upstream_0 into master
1 unresolved thread

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • While properly distinguishing DXBC and TPF seems a good idea, I am not a fan of having very long source files which mix different logical pieces of code. I find them difficult to read and to understand. So I'd keep separate the parsing and emitting code, both for SM4 and SM1.

    (also, I find a bit funny the way we handle copyright lines in headers: it seems they are added by the first person checking in a file and never updated any more; not that it's a big problem for me, just funny)

  • Author Maintainer

    I wouldn't call either d3dbc.c or tpf.c very long, but I suppose that's a matter of opinion. I imagine it doesn't help that I find the practice of spreading source code over a million tiny files to be one of the most irritating things a codebase can do...

    In any case, while it's perhaps not quite true at the moment, parsing and serialising the different bytecode formats should be closely related and use the same constants, structures, and so on. There's currently some HLSL-specific code intermixed that would more properly belong in hlsl.c/hlsl_codegen.c, but we can deal with that once we get to it.

  • added 12 commits

    • 4901a6b5...e34bdcab - 8 commits from branch wine:master
    • 804e315b - vkd3d-shader/sm4: Move the TPF parser from dxbc.c to hlsl_sm4.c.
    • 7800c7b4 - vkd3d-shader/sm4: Rename hlsl_sm4.c to tpf.c.
    • 99bc07cc - vkd3d-shader/sm4: Merge sm4.h into tpf.c.
    • 8e0df3f7 - vkd3d-shader/sm1: Merge hlsl_sm1.c into d3dbc.c.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Oof, I would have asked that we not do this quite yet :-/

    I wouldn't call either d3dbc.c or tpf.c very long, but I suppose that's a matter of opinion. I imagine it doesn't help that I find the practice of spreading source code over a million tiny files to be one of the most irritating things a codebase can do...

    I suppose it depends on your standard for how long a file can be. I feel like more than a few thousand lines can start to feel like an awful lot, and at that point the compilation time does start to show.

    Personally I think modularity is the most important thing, and I find it somehow easier to mentally work with files that segregate their components. In this case I think segregating the reader and writer would not have been a bad idea, and keeping the sm4 definitions in a separate file might also have been nice even if not.

    Obviously by that principle, any sufficiently modular piece of code can be split into its own file, however small. If it's only a few hundred lines, or a couple of functions (honestly, the number of [well-formed?] functions may matter less to me than the line count) I'd be inclined against it, but I think I have a lower threshold for what seems reasonable to split up. Take HLSL copy propagation, for instance, which is only about 500 lines, but on the other hand spans a whole 20 functions; that feels to me like a large enough self-contained chunk of code that it's worth at least considering segregation.

    In any case, while it's perhaps not quite true at the moment, parsing and serialising the different bytecode formats should be closely related and use the same constants, structures, and so on. There's currently some HLSL-specific code intermixed that would more properly belong in hlsl.c/hlsl_codegen.c, but we can deal with that once we get to it.

    ...Possibly. I would have appreciated a bit more discussion on this first. I had been vaguely thinking of moving more code to the backend files, so now I need to stop and re-plan.

    And while I think we'd always been talking about using the same structures in hlsl_smX, I kind of thought that we would perform that conversion before moving things around. That would have let things be done more gradually, and avoided some unpleasant rebases.

    Anyway, generally this ties into the question I brought up in [1]. I brought up a relatively complete summary of our options with the IR there, but it doesn't seem to have garnered any discussion. At best we seem to have committed to a "maximally CISC" instruction set with Conor's normalization patches, so that rules out option (2).

    I'd obviously like to keep HLSL details out of the sm1/sm4 code, but I'd also really like to keep sm1/sm4 details out of hlsl_codegen.c as much as we can. We are still going to need specific hlsl -> smX glue for some things like register allocation and semantic validation, and that may be unavoidable (though on the other hand, a lot of that may end up being shareable... certainly in the case of register allocation it currently is; I'm just not sure if it should remain that way.)

    Past that, it's still an open question what kind of instructions the HLSL compiler should actually emit. I don't really know the answer to this one. My natural inclination is that we want the HLSL compiler to care about its target as little as possible [so I'm inclined against option (1)].

    Going further, even if the IR is going to be maximal anyway, I'd say that HLSL should probably still emit a more minimal subset [basically option (3)], and then we would have backend-specific optimization passes that "raise" simple IR into more complex IR. We don't want to complexify the HLSL IR to match the CISC capabilities of vkd3d_shader_instruction, and if we're going to have to perform "raising" passes on vkd3d_shader_instruction anyway, probably better to raise them per backend.

    Of course, there are upsides and downsides to each approach, and I'm not fully sure which is best.

    [1] !37 (comment 13059)

    • Author Maintainer

      I suppose it depends on your standard for how long a file can be. I feel like more than a few thousand lines can start to feel like an awful lot, and at that point the compilation time does start to show.

      Personally I think modularity is the most important thing, and I find it somehow easier to mentally work with files that segregate their components. In this case I think segregating the reader and writer would not have been a bad idea, and keeping the sm4 definitions in a separate file might also have been nice even if not.

      Obviously by that principle, any sufficiently modular piece of code can be split into its own file, however small. If it's only a few hundred lines, or a couple of functions (honestly, the number of [well-formed?] functions may matter less to me than the line count) I'd be inclined against it, but I think I have a lower threshold for what seems reasonable to split up. Take HLSL copy propagation, for instance, which is only about 500 lines, but on the other hand spans a whole 20 functions; that feels to me like a large enough self-contained chunk of code that it's worth at least considering segregation.

      I don't want to be insensitive to the concern, particularly since both Giovanni and you raised it, but I just don't get it? Navigating within a file seems at least as easy as navigating between files, and files on the order of ~2k lines in case of d3dbc.c and ~5k lines in case of tpf.c don't seem particularly unusual in either vkd3d or Wine. (I had been thinking we might as well merge hlsl.c, hlsl_codegen.c and hlsl_constant_ops.c, but I guess not then...)

      In any case, while it's perhaps not quite true at the moment, parsing and serialising the different bytecode formats should be closely related and use the same constants, structures, and so on. There's currently some HLSL-specific code intermixed that would more properly belong in hlsl.c/hlsl_codegen.c, but we can deal with that once we get to it.

      ...Possibly. I would have appreciated a bit more discussion on this first. I had been vaguely thinking of moving more code to the backend files, so now I need to stop and re-plan.

      And while I think we'd always been talking about using the same structures in hlsl_smX, I kind of thought that we would perform that conversion before moving things around. That would have let things be done more gradually, and avoided some unpleasant rebases.

      Anyway, generally this ties into the question I brought up in [1]. I brought up a relatively complete summary of our options with the IR there, but it doesn't seem to have garnered any discussion. At best we seem to have committed to a "maximally CISC" instruction set with Conor's normalization patches, so that rules out option (2).

      Well, yeah, the impression I came away with from that conversation was that there were no particularly strong preferences, and not much of a general consensus either; few people seem to care. I don't think we've ruled out any of the options presented there at this point. As noted there, (1) is essentially the current state, but we could certainly move away from that towards one of the other options.

      And ultimately I think the distinction between the options is a bit artificial to begin with; it ends up largely being a matter of doing particular transformation passes on either the frontend IR, the intermediate IR, or the backend IR. That's not a decision we need to make upfront for every conceivable transformation, and we can change our minds if a particular choice doesn't quite work out. In the case of Conor's tessellation shader work, the consideration is simply that these transformations are more practical to do on the vkd3d_shader_instruction level than on the SPIR-V level, and while doing them on the TPF level would also be plausible, that would require doing a bunch of disassembler work first.

    • I don't want to be insensitive to the concern, particularly since both Giovanni and you raised it, but I just don't get it? Navigating within a file seems at least as easy as navigating between files, and files on the order of ~2k lines in case of d3dbc.c and ~5k lines in case of tpf.c don't seem particularly unusual in either vkd3d or Wine. (I had been thinking we might as well merge hlsl.c, hlsl_codegen.c and hlsl_constant_ops.c, but I guess not then...)

      I suppose it depends a lot on one's own thought processes and habits, so I don't claim that what's better for me is intrinsecally better for everybody. Also, as you say, Wine's standards are much worse than what we're discussing here, so, while I don't think they are good standards, I had to come to terms with them.

      I agree with Zeb that having smaller files helps segregation of code, and if code is segregated properly with reasonable interfaces between the segregated parts it is easier (for me at least) to understand and change, because I can study each of the segregated parts somewhat independently. In theory it is not the number of lines or functions that counts, but in practice, with some exceptions, once a file has grown past a few thousands lines of code it's hard for me not to think that it would make good use of some splitting.

      For example, I would find ideal to have hlsl_codegen.c splitted in a few pieces. The prepend/append copies and copy propagation sections could be easily moved to an independent file each one, keeping basically everything static except a handful of functions that are called by hlsl_emit_bytecode(). You have a small interface, but everything else you have to do in copy propagation doesn't need to interact with all the rest of hlsl_codegen.c, and splitting the file ensures that. If somebody wants to learn how copy propagation works, they can read hlsl_copy_propagation.c instead of having to potentially have to go through the whole of hlsl_codegen.c. This luckily already happened for hlsl_constant_ops.c. I wouldn't move each other pass in its own file because that would be too small, though. There is some sweet spot, which of course can change from person to person.

      As for tpf.c, the parser and writer parts are basically completely independent. They don't even share the internal representation. When I am working on the parser it's unlikely that I care about the writer at all. So having them in the same place is just confusing for me. Since they need to share some code (mostly the definitions) it makes sense to move that in a shader header, as it was before. If, say, I want to refactor how an hypothetical field type is used, then I'm likely to search through the file for that identifier. And if unrelated code sits in the same file, I'll have a lot more false positives.

      As I said above, that's probably a very personal thing, and having bigger files is not a blocker for me. Just wanted to share my point of view.

    • I don't want to be insensitive to the concern, particularly since both Giovanni and you raised it, but I just don't get it? Navigating within a file seems at least as easy as navigating between files, and files on the order of ~2k lines in case of d3dbc.c and ~5k lines in case of tpf.c don't seem particularly unusual in either vkd3d or Wine. (I had been thinking we might as well merge hlsl.c, hlsl_codegen.c and hlsl_constant_ops.c, but I guess not then...)

      I suppose it depends a lot on one's own thought processes and habits, so I don't claim that what's better for me is intrinsecally better for everybody. Also, as you say, Wine's standards are much worse than what we're discussing here, so, while I don't think they are good standards, I had to come to terms with them.

      I agree with Zeb that having smaller files helps segregation of code, and if code is segregated properly with reasonable interfaces between the segregated parts it is easier (for me at least) to understand and change, because I can study each of the segregated parts somewhat independently. In theory it is not the number of lines or functions that counts, but in practice, with some exceptions, once a file has grown past a few thousands lines of code it's hard for me not to think that it would make good use of some splitting.

      Something like this. I find it hard to give an objective rationalization of why I want to split things up, but I think to a degree it comes down to the same reasons that large functions should be split up. It communicates to the reader that a piece of code is self-contained and should make sense by itself. On the other hand, when I see a large file, much like a large function, I can't help but think "can this be split up into a coherent set of pieces?" To a degree I guess it also is an intuitive judgement, and that can be poor.

      The cost of compiling large files shouldn't be discarded either, I don't think. E.g. our d3d12.c test file (or d3d11.c, and so on) doesn't really need to be split from an architectural standpoint—yes, the tests can stand alone, but there's no common thread segregating any one group of tests from another, and splitting every test into its own file seems excessive. No, if something makes me want to split up that file, it's that it takes multiple seconds to compile (depending on machine).

      I don't think there's any concern regarding navigation, at least on my part.

      That said, it's also something that feels like a waste of time to argue a lot about. I didn't feel like hlsl_constant_ops.c was particularly necessary, and would probably have just kept it in hlsl_codegen.c if it was just me, but it also doesn't feel like it's worth bikeshedding at all. Nor do I really intend to argue about splitting up the backend readers and writers here, just make a vague comment about code organization and move on.

    • In any case, while it's perhaps not quite true at the moment, parsing and serialising the different bytecode formats should be closely related and use the same constants, structures, and so on. There's currently some HLSL-specific code intermixed that would more properly belong in hlsl.c/hlsl_codegen.c, but we can deal with that once we get to it.

      ...Possibly. I would have appreciated a bit more discussion on this first. I had been vaguely thinking of moving more code to the backend files, so now I need to stop and re-plan.

      And while I think we'd always been talking about using the same structures in hlsl_smX, I kind of thought that we would perform that conversion before moving things around. That would have let things be done more gradually, and avoided some unpleasant rebases.

      Anyway, generally this ties into the question I brought up in [1]. I brought up a relatively complete summary of our options with the IR there, but it doesn't seem to have garnered any discussion. At best we seem to have committed to a "maximally CISC" instruction set with Conor's normalization patches, so that rules out option (2).

      Well, yeah, the impression I came away with from that conversation was that there were no particularly strong preferences, and not much of a general consensus either; few people seem to care. I don't think we've ruled out any of the options presented there at this point. As noted there, (1) is essentially the current state, but we could certainly move away from that towards one of the other options.

      Yeah, that makes sense. It's just that there are several vkd3d developers rather experienced in design—and also often opinionated—and I was hoping to get at least some thoughts on the matter :-/

      And ultimately I think the distinction between the options is a bit artificial to begin with; it ends up largely being a matter of doing particular transformation passes on either the frontend IR, the intermediate IR, or the backend IR. That's not a decision we need to make upfront for every conceivable transformation, and we can change our minds if a particular choice doesn't quite work out. In the case of Conor's tessellation shader work, the consideration is simply that these transformations are more practical to do on the vkd3d_shader_instruction level than on the SPIR-V level, and while doing them on the TPF level would also be plausible, that would require doing a bunch of disassembler work first.

      I think it's less artificial than it looks, largely because we only have the one target (SPIRV) that would involve lowering transformations. Which is to say, there are a lot of lowering transformations that we could be doing, rather than (as I suspect we will initially) independently lowering things in GLSL and SPIRV. Similarly there are a lot of raising transformations we could be doing in HLSL -> smX that we currently aren't.

      I think there's also a lot to be said for being consistent about where we do a given transformation, but maybe I'm cargo-culting consistency a bit too hard.

      The effect of all this is that while it's not hard to adjust, it could eventually involve rewriting a lot of transformations, and so it seems like the kind of thing to think about now before we commit to one approach.

    • For example, I would find ideal to have hlsl_codegen.c splitted in a few pieces. The prepend/append copies and copy propagation sections could be easily moved to an independent file each one, keeping basically everything static except a handful of functions that are called by hlsl_emit_bytecode(). You have a small interface, but everything else you have to do in copy propagation doesn't need to interact with all the rest of hlsl_codegen.c, and splitting the file ensures that. If somebody wants to learn how copy propagation works, they can read hlsl_copy_propagation.c instead of having to potentially have to go through the whole of hlsl_codegen.c. This luckily already happened for hlsl_constant_ops.c. I wouldn't move each other pass in its own file because that would be too small, though. There is some sweet spot, which of course can change from person to person.

      Mesa does this, and I suspect this is exactly the kind of thing that Henri finds distasteful. Personally it seems excessive to me as well, actually. There is a cost to having a lot of files, mostly in terms of slowing navigation a bit, and having to have more tabs open in one's editor.

    • Yeah, that makes sense. It's just that there are several vkd3d developers rather experienced in design—and also often opinionated—and I was hoping to get at least some thoughts on the matter :-/

      I don't consider myself experienced, at least not into IR design. Until little time ago I knew basically nothing about vkd3d_shader_instruction, and still now I don't know that much. Among other things, this means that it's still not completely clear to me what you mean by "maximally CISC" or "minimally CISC" (and while I think I sort of get the general idea for the terms, that's not nearly enough to understand options (3) and (4)). Therefore I don't feel I have a definite opinion about the subject. There are just a couple of rather vague points that come to my mind.

      • In general, I think that code processing should be done in the form of IR passes as much as possible, rather than be embedded in the frontends or backends. This helps modularity and code sharing. I think Conor's patches go into this direction, which makes sense to me. Frontends and backends already care about serialization and deserialization and should not be loaded with excessive other duties.

      • Currently my understanding is that vkd3d_shader_instrucion is basically modeled after SM4. When converting SM4 -> SPIR-V, the SM4 code is basically deserialized to vkd3d_shader_instruction and then rewritten to SPIR-V in a rather naive way. The deserialization step is very syntactical, to the point that the original SM4 code can be faithfully disassembled from the IR. If we want vkd3d_shader_instruction to be flexible enough to support different frontends and backends, I think it must somehow be unchained from SM4. In particular, SM4 disassembling has to go through a different path. An option is to have an IR specific for SM1/SM4 (which are probably close enough to have the same IR) and another more general IR, that is meant to achieve interoperability for many different shader languages. The SM4-IR would be rather similar to the current vkd3d_shader_instruction (so syntactically close to SM4), and the "general" IR could be more like an actual compiler IR (SSA, possibly more RISC, if I understand Zeb's terminology). If eventually we want to do some SPIR-V-specific optimizations, there could be the need to a SPIR-V-IR too. The HLSL compiler could emit general-IR (which would be somewhat similar to what's already emitting), which would then be optimized, converted to SM4-IR (that's possibly similar to a direction that was already considered), optimized again (for passes that go not make sense for general-IR) and then emitted (which would be basically serializing). It's quite a lot of work, anyway.

      As I said, take all of this with a grain of salt. Maybe two grains. Ok, it probably wouldn't hurt to add a cargo ship's worth of salt.

    • Among other things, this means that it's still not completely clear to me what you mean by "maximally CISC" or "minimally CISC" (and while I think I sort of get the general idea for the terms, that's not nearly enough to understand options (3) and (4)).

      Ah, I'm sorry. To put it into more concrete terms: HLSL is intentionally a quite simple IR. I would describe it as erring on the side of RISC in design. Everything operates on SSA values except for extern and resource loads; single instructions do pretty much maximally modular things; we avoid adding new expression types or instruction types if we can instead just lower them to simpler expressions immediately.

      By contrast, both sm1 and sm4 are more CISC. One instruction can do multiple arithmetic operations (abs, neg, sat) in addition to whatever else it's doing; instructions load directly from multiple types of registers (instead of always going through an SSA value, or even just always going through a temporary register).

      Part of being able to make statements like "HLSL IR is RISC" is the knowledge that it used to be less so (e.g. we used to have HLSL_IR_CONSTRUCTOR, which I assume you can guess the idea of) and also that we've considered making it less so. On the basis that one HLSL instruction corresponds to one smX instruction we've considered adding some of those features to HLSL IR (for example, making hlsl_src hold a union that includes not just SSA values but also immediate constants or something similar to hlsl_deref). We eventually decided against those on the grounds that it would make the IR more complex, and harder to reason about when doing optimization passes.

      • In general, I think that code processing should be done in the form of IR passes as much as possible, rather than be embedded in the frontends or backends. This helps modularity and code sharing. I think Conor's patches go into this direction, which makes sense to me. Frontends and backends already care about serialization and deserialization and should not be loaded with excessive other duties.

      Yeah. Well, in a sense the question can be rephrased as: should v-s IR exist just as a common, "neutral" format, and then we'd have basically frontend- and/or backend-specific IR that we'd do passes over? E.g. in this case the backend IR would probably be adapted from struct sm4_instruction, which currently exists just to be a slightly more structured version of the byte code, but could grow to be more than that. It seems we probably won't be moving this way, but that's kind of what I was envisioning with that proposal. In that case struct vkd3d_shader_instruction wouldn't have any optimization passes done over it, the only raison d'être would be to help allow mixing any frontend with any backend.

      • Currently my understanding is that vkd3d_shader_instrucion is basically modeled after SM4. When converting SM4 -> SPIR-V, the SM4 code is basically deserialized to vkd3d_shader_instruction and then rewritten to SPIR-V in a rather naive way. The deserialization step is very syntactical, to the point that the original SM4 code can be faithfully disassembled from the IR.

      I believe that vkd3d_shader_instruction is modeled after (or even "adapted directly from") struct wined3d_shader_instruction, which was designed to handle both sm1 and sm4. Fundamentally the formats are relatively similar, enough that it's possible to write a single disassembly routine, and a single GLSL shader backend, that mostly handles both, although there did need to be a lot of version-specific code in the latter case.

      If we want vkd3d_shader_instruction to be flexible enough to support different frontends and backends, I think it must somehow be unchained from SM4. In particular, SM4 disassembling has to go through a different path.

      Right, and there's part of the rub. If we want it to be usable for things like disassembly (and assembly), we need v_s_i to be able to express everything that any frontend or backend can. This ends up kind of bloating the structure, which is one of the reasons I'm not sure we want that "maximally CISC" kind of IR.

      Like I mentioned, the less complicated an IR is, the easier it is to work with. On the other hand, the kind of passes we'd be potentially doing over a maximally CISC v_s_ir aren't the same as the work we do with HLSL IR. HLSL has to bridge the gap from text all the way down to byte code, but v_s_ir would potentially just be a bunch of peepholes. The fact that it doesn't have to deal with types, or well, doesn't have to deal with data structures, is already quite a benefit. So I'm not sure anymore that that per se is a concern.

      And of course when trading off one complex IR against multiple (ideally less complex) IRs, it takes some judgement to decide which is the best option.

    • Thanks for the explanation. Keeping in mind my remark about salt and cargo ships from yesterday, here are my thoughts.

      If we want it to be usable for things like disassembly (and assembly), we need v_s_i to be able to express everything that any frontend or backend can.

      And I don't think we want it, it would be unmanageable.

      should v-s IR exist just as a common, "neutral" format, and then we'd have basically frontend- and/or backend-specific IR that we'd do passes over?

      Ideally this would feel a good design to me. Though I would say that it is not a hard requirement for frontends and backends to have their specific IR, even if it is conceivable that past a certain complexity threshold it is difficult to do without. Also, my idea is that you can do passes over {front,back}end specific IRs, but also over the common IR. At least, I'd design the common IR in such a way that they can be done.

      In practice I see a number of difficulties:

      • Shaders are not just code; they also embed interface information (input and output signatures, uniforms, buffers, object bindings, ...) and other random metadata (for example for the hull shaders). Should this be representable in the common IR?

      • How essential (or "RISC") should the IR be? Should it be scalar or vector? Making it more essential means that frontends have to do more work and common IR passes are easier to write; make it less means that backends have to do more work, though it also has the advantage that passes that make sense for different frontends and backends can be deduplicated.

      Maybe I am just rehashing the same elements around and around. It's quite hard to design this thing...

    • If we want it to be usable for things like disassembly (and assembly), we need v_s_i to be able to express everything that any frontend or backend can.

      And I don't think we want it, it would be unmanageable.

      should v-s IR exist just as a common, "neutral" format, and then we'd have basically frontend- and/or backend-specific IR that we'd do passes over?

      Ideally this would feel a good design to me. Though I would say that it is not a hard requirement for frontends and backends to have their specific IR, even if it is conceivable that past a certain complexity threshold it is difficult to do without. Also, my idea is that you can do passes over {front,back}end specific IRs, but also over the common IR. At least, I'd design the common IR in such a way that they can be done.

      This is what I originally thought, but I'm having slight second thoughts now. The thing is that the tradeoff is now you need a new IR for every bytecode language you have, which in our case means at least sm1, sm4, and eventually I think sm6. That's a lot of extra support code to add. Consider how much we'd need to add by putting that intermediate step into HLSL -> smX translation. The advantage of making v_s_i the IR is that you don't actually need any of that.

      It does feel conceptually ugly to have a CISC IR, but looking past that, I'm not sure it's that bad? Some additions (new opcodes, new register types) aren't a problem, since the backends can just vkd3d_shader_error(). Some additions (new source fields, new instruction flags) are worse, though, since now you need to make sure that nothing else cares...

      It may also not be that much of a concern if the number of languages we have doesn't actually grow that large.

      It may make sense to start with something underdesigned—which I think means CISC—and if that starts to get actually unwieldy, then we can start overdesigning a RISC IR to mediate.

      In practice I see a number of difficulties:

      • Shaders are not just code; they also embed interface information (input and output signatures, uniforms, buffers, object bindings, ...) and other random metadata (for example for the hull shaders). Should this be representable in the common IR?

      We need some way to represent that information so it doesn't get lost, at least. I think we don't want side channels, to be clear: I think that there should be a point where all of the information relevant to a shader is represented in v_s_i format, including metadata.

      And contrary to the instructions themselves, I think we want this information to be in a relatively simple and well-designed form. In a sense, it doesn't need to be "complex" like the instructions since we're not doing optimization passes over it [if that makes sense]. I think Conor has basically been pushing v_s_i in this direction in order to accommodate sm6 (at least wrt semantics), and given the tribulations I've had to endure to get sm1 to work, I think that's the right approach.

    • This is what I originally thought, but I'm having slight second thoughts now. The thing is that the tradeoff is now you need a new IR for every bytecode language you have, which in our case means at least sm1, sm4, and eventually I think sm6. That's a lot of extra support code to add. Consider how much we'd need to add by putting that intermediate step into HLSL -> smX translation. The advantage of making v_s_i the IR is that you don't actually need any of that.

      You mean that you'd have just v_s_i and every frontend and backend deals directly with it, including for very syntactical things like assembling and disassembling? I don't feel really convinced, it seems to me that you'd have to encode su much stuff in v_s_i that each time you need to change something (for example to allow for a new language) the changes can have repercussions in each other language frontend or backend.

    • Please register or sign in to reply
  • Author Maintainer

    The effect of all this is that while it's not hard to adjust, it could eventually involve rewriting a lot of transformations, and so it seems like the kind of thing to think about now before we commit to one approach.

    Right, I don't disagree; I'm generally in favour of thinking things through. What I'm saying is that right now we haven't committed to a particular approach yet, largely because there doesn't appear to be a clear winner; it mostly just seems like different sets of trade-offs at this point. If you have a preference, and want to have a go at moving things in that direction, by all means go ahead. :)

  • Author Maintainer

    In practice I see a number of difficulties:

    • Shaders are not just code; they also embed interface information (input and output signatures, uniforms, buffers, object bindings, ...) and other random metadata (for example for the hull shaders). Should this be representable in the common IR?

    We need some way to represent that information so it doesn't get lost, at least. I think we don't want side channels, to be clear: I think that there should be a point where all of the information relevant to a shader is represented in v_s_i format, including metadata.

    And contrary to the instructions themselves, I think we want this information to be in a relatively simple and well-designed form. In a sense, it doesn't need to be "complex" like the instructions since we're not doing optimization passes over it [if that makes sense]. I think Conor has basically been pushing v_s_i in this direction in order to accommodate sm6 (at least wrt semantics), and given the tribulations I've had to endure to get sm1 to work, I think that's the right approach.

    Yes, although I don't think that necessarily means storing these as "instructions". The approach I'm personally leaning towards at this point is to rebrand "struct vkd3d_shader_instruction_array" as "struct vkd3d_shader_program" (or something along those lines), and then add the input/output/patch constant signatures and the shader version structure to that structure. I think a similar approach should work for e.g. RDEF as well.

  • In practice I see a number of difficulties:

    • Shaders are not just code; they also embed interface information (input and output signatures, uniforms, buffers, object bindings, ...) and other random metadata (for example for the hull shaders). Should this be representable in the common IR?

    We need some way to represent that information so it doesn't get lost, at least. I think we don't want side channels, to be clear: I think that there should be a point where all of the information relevant to a shader is represented in v_s_i format, including metadata.

    And contrary to the instructions themselves, I think we want this information to be in a relatively simple and well-designed form. In a sense, it doesn't need to be "complex" like the instructions since we're not doing optimization passes over it [if that makes sense]. I think Conor has basically been pushing v_s_i in this direction in order to accommodate sm6 (at least wrt semantics), and given the tribulations I've had to endure to get sm1 to work, I think that's the right approach.

    Yes, although I don't think that necessarily means storing these as "instructions". The approach I'm personally leaning towards at this point is to rebrand "struct vkd3d_shader_instruction_array" as "struct vkd3d_shader_program" (or something along those lines), and then add the input/output/patch constant signatures and the shader version structure to that structure. I think a similar approach should work for e.g. RDEF as well.

    Yes, agreed wholeheartedly there.

  • Author Maintainer

    This is what I originally thought, but I'm having slight second thoughts now. The thing is that the tradeoff is now you need a new IR for every bytecode language you have, which in our case means at least sm1, sm4, and eventually I think sm6. That's a lot of extra support code to add. Consider how much we'd need to add by putting that intermediate step into HLSL -> smX translation. The advantage of making v_s_i the IR is that you don't actually need any of that.

    You mean that you'd have just v_s_i and every frontend and backend deals directly with it, including for very syntactical things like assembling and disassembling? I don't feel really convinced, it seems to me that you'd have to encode su much stuff in v_s_i that each time you need to change something (for example to allow for a new language) the changes can have repercussions in each other language frontend or backend.

    Probably not for everything. E.g., I suspect the DXIL disassembler probably shouldn't go through the common IR. But fundamentally, I think that's much of the point of having a common IR. DXIL support, though not quite done yet, is perhaps a decent example there. Without going through the common IR, that would require duplicating a decent chunk of the code currently in spirv.c. There's some work required upfront to make the common IR suitable for DXIL, but ultimately the resulting code should be more maintainable.

  • You mean that you'd have just v_s_i and every frontend and backend deals directly with it, including for very syntactical things like assembling and disassembling? I don't feel really convinced, it seems to me that you'd have to encode su much stuff in v_s_i that each time you need to change something (for example to allow for a new language) the changes can have repercussions in each other language frontend or backend.

    Yeah, that's the primary concern.

    It's mostly at this point weighing that against the idea of introducing new IRs and translation passes for everything, and also writing specific assemblers and disassemblers for everything, that makes me think keeping v_s_i as complex as it is isn't actually that bad.

    FWIW, it's not necessarily that everything would go directly into v_s_i (clearly HLSL wouldn't), but sm1 and sm4 at least would. Contrary to what Henri said, I was actually of the impression that sm6 could deal directly with v_s_i without adding too much? Besides new opcodes and interfaces, I thought we didn't need much more than a new "SSA" register type.

  • Author Maintainer

    FWIW, it's not necessarily that everything would go directly into v_s_i (clearly HLSL wouldn't), but sm1 and sm4 at least would. Contrary to what Henri said, I was actually of the impression that sm6 could deal directly with v_s_i without adding too much? Besides new opcodes and interfaces, I thought we didn't need much more than a new "SSA" register type.

    Well, there's the shader I/O signature and register normalisation work that Conor is currently working on, for example. I don't know if I'd qualify that as "much more", but it's essentially adapting the TPF frontend and SPIR-V backend to accommodate the DXIL frontend. It doesn't touch the representation of individual instructions much, so in that sense, sure. (Also, it ends up actually simplifying the SPIR-V backend by moving some transformations that aren't needed for DXIL to the common IR level where they're easier to do...)

Please register or sign in to reply
Loading