On Mon Jul 3 18:26:07 2023 +0000, Zebediah Figura wrote:
I don't know if I can describe why in any exact terms, but it doesn't feel as easy to understand to me.
Generally I prefer the new approach (store the iteration instruction
list and then copy it directly before each `continue`) rather than the old one (first generate code without the iteration instructions and then patch it). One of the reasons is that, as you said, it's good that the IR has a well-defined meaning independently of the processing stage. I guess. It might help if we were to arrange the rules more like loop_statement: loop_header statement loop_header: attribute_list_optional KW_FOR '(' scope_start expr_statement expr_statement expr_optional ')' and then loop_header can essentially call create_loop() as now but without filling the body yet, then loop_statement would take care of moving the "statement" body into the loop instruction. That would also remove the need for "loop_scope_start". Hmm, but this breaks with do-while...
Okay, so a minute later I realize that there's a problem which I think prevents this approach from working in the first place. 'continue' doesn't just need to apply the iterator, it needs to apply the conditional in the case of do-while loops, and we can't know what the conditional is until we've parsed the whole loop. [This is only a concern for do-while; for other loop types the conditional is executed at the head of the loop.] Example shader, sorry it's a bit complex:
uint m; float4 main() : sv_target { float4 f = 0; uint x = 0; do { if (x % 2 == 0) continue; f += 5; } while (x++ < m); return f; }compiled with sm4 you can see it increments x on every iteration. So we either can fix up loops in create_loop(), or we can fix them up in a separate pass. In either case, for the sake of having a consistent meaning, it probably does make sense to have two separate "continue" jump types, though I'm not sure how to name them.
Ugh, that's right. I hate that. I would name the new jump type something like `ITER_AND_CONTINUE`. The annoying detail is that that instruction has no meaning unless you know what the iteration block is, so it still happens that if you look at a IR dump is `ITER_AND_CONTINUE` you don't know which program is really being described. That instruction only make sense as long as you're inside the context in which the loop is being parsed, and at this point it doesn't look much better than just using `CONTINUE`, even if that's the solution that I originally disliked.
Unfortunately I don't have any better solution in mind for the "do while" loop. I can't exclude there might be some clever trick with bison to pre-parse the loop body, than parse the "while" condition and then parse the body for real, but even if there was I'm not sure it would be worth the additional complexity of the parser.
In conclusion, I think the best way to go is the original solution. I don't have a strong preference between overriding the `CONTINUE` jump, or introduce a new jump type.