On Thu Oct 26 20:47:13 2023 +0000, Zebediah Figura wrote:
I've been sitting on this because I'm still not sure about 4/9 and 5/9, when we're going to be feeding this through a lower level IR that's better equipped to handle those kinds of transformations. I still feel like this is going in the wrong direction. I don't want to block implementing an important feature forever, but I also don't want that refactoring to never happen. I suppose I can accept this as long as we *are* committed to fixing this the right way eventually, and not putting it off forever just because it's hard. I'm also bothered by 3/9. hlsl_deref_is_lowered() is used in two places:
- hlsl_deref_get_type(), which feels like it should just have "if
(deref->data_type) return deref->data_type;"
- dump_deref(), which I'm not sure why it needs to call that function at
all? Why isn't that just "else"?
Alright, I will work on lowering HLSL to vsir in the following months, probably trying to conquer that bridge from both sides.
From what we talked about, on one part we want tpf.c and d3dbc.c to not receive any HLSL IR, and on the other we want hlsl_codegen.c to output vsir. And we will probably will want two passes that translate from general vsir to the specific subset of vsir compatible with these formats, and the validation passes too.
Regarding `hlsl_deref_is_lowered()`, I would like to keep it since it doesn't hurt (doesn't require ctx), its name makes is clear what we are checking for, instead of just checking for `deref->data_type` (which wouldn't be enough if we had accepted !343).
Additionally, in further passes I introduce many asserts like ``` assert(!hlsl_deref_is_lowered(lhs)); ```
- hlsl_deref_get_type(), which feels like it should just have "if (deref->data_type) return deref->data_type;"
We can do that, but the current version has the benefit of explaining that deref->data_type is not used if the deref is not lowered yet. That deref->data_type is the way that we check that is just a coincidence.
*dump_deref(), which I'm not sure why it needs to call that function at all? Why isn't that just "else"?
Because there is the possibility of a path deref to have path_len=0, if it is a direct reference to the variable, and I think we don't want to write those as `[0]` or `[[]]`.
Well, this is a pretty small thing, I can totally replace the hlsl_deref_is_lowered() calls with deref->data_type checks if you want.