Also, I'd like to stress this because it has come up before for other MRs, please don't pre-emptively add complications for issues that will only come up later; whether that's in a later patch in the same series or 400 patches later in the branch you're upstreaming from. Reviewers generally can't or don't want to look that far ahead, and it just ends up slowing the entire process down. Patches should make sense in isolation, at the point in time where they're introduced.
I agree strongly with this in general. I find myself often asking people to do this. But—and I'm sure I'm missing some perspective here—I don't see it as being a problem in this specific case.
Well, part of the context is at which point in the release process we are. We were about a week into the freeze when this was first submitted, and we're about a week from the release now. If this had been submitted in e.g. the first month or so after the 1.8 release, I would probably still have grumbled about the process and posted a comment along the lines of "please do better", but it may have ultimately gone in. As it is, I don't think so, and for the most part that's simply because the MR is touching things outside dxil.c that it doesn't strictly have to touch.
Prematurely adding complications is a problem, I think, when I (as a reviewer) can't see how they're going to be used, and when I feel that there might be a better, simpler option that will possibly preclude the more complex code from *ever* being necessary.
I don't see an issue with the first part; I have the barest of knowledge about sm6 but I can tell that if it has SSA nodes, then we're introducing a vsir SSA node to translate from sm6 SSA nodes and that it's going to be used to translate to spirv SSA nodes.
As for whether there's a better, simpler option—I have a hard time personally seeing temps as simpler.
Which parts of the system are you considering when you make that evaluation? Introducing SSA nodes is certainly simpler if you consider dxil.c in isolation, or even the dxil -> spirv path in isolation. That's (trivially) not true when you consider e.g. d3d_asm.c or spirv.c in isolation. Similarly, SSA nodes are extra work for someone adding e.g. a MSL or GLSL backend. What about transformation passes in ir.c? Are those potentially going to need to handle VKD3DSPR_SSA? We don't run any of the existing ones on dxil sources, but that doesn't make the consideration just go away.
It gets more complicated when you consider vkd3d-shader as a whole. We add some complexity in some places, but avoid some in others. We probably also eventually want SSA nodes for the HLSL compiler anyway. Is that worth it in the end? Well, probably.
What we're concerned about here though, is review/upstreaming complexity. And it's not even close there; SSA nodes introduce the evaluation above, as well as e.g. considerations about whether it's prudent to introduce these at this point in the release process, while for changes confined to dxil.c it would be only slight more complicated than "Well, we didn't support dxil before, so it's not terribly likely to make things worse."
This is partly because the SSA register type is just not complicated to begin with, and I don't expect the sm6 -> vsir or vsir -> spirv translations thereof to be complicated either. But it's also because passes on vsir (both the ones I'm anticipating related to sm1/sm4 output, and the ones Conor is talking about) are going to be distinctly harder to do on temps than on SSA nodes. [Some of this is armchair analysis, some of this may be bias from having just written the code, but some of it is influenced by comparing copy-prop to our other HLSL passes. Copy-prop is more complex and harder to reason with in general, and I think roughly models how we'd have to do passes on non-SSA.]
(And, well, I'm more than a little concerned about compilation speed, and compiled shader size, which makes me think that we likely *will* SSA nodes in vsir at some point. But I don't have any data to back that up, so I'll let go of that concern for now...)
Sure; I don't think anyone raised concerns about introducing SSA nodes per se. The issue is mostly about whether the rest of this MR (and by extension future dxil patches) should depend on that (and by extension the 1.9 release).