Rework the API for outgoing blockparams#18
Conversation
cfallin
left a comment
cfallin
left a comment
There was a problem hiding this comment.
Thanks for this! I was unsure at first about this refactor (the appeal of the blockparams approach to me is that phis turn into normal operands at their points of logical use, but... now they aren't operands?) but I think this is actually the right abstraction, on further thought: blockparams are indeed differently-constrained (they aren't really a use that must read the value, they just plumb it through), and the scheme for packing them contiguously in the args of a branch instruction was awkward. And blockparam inputs are special, so why not blockparam outputs too?
IIRC, an earlier iteration of RA2 actually depended on the uses existing, but the current implementation of blockparam plumbing makes use of the unified move generation so I think everything should work as it is now. Just to check: did you run the fuzzer for a bit with the updated logic?
|
I did run the fuzzer, but looking at the code again I think that the live ranges are not being tracked properly. I need to double-check this. |
|
Ah, yes, the dataflow analysis around |
|
Fixed, but I'll leave the fuzzer running overnight again to be sure. |
|
Fuzz tested ~4M test cases and no problems found! But then again fuzzing was also passing without the liverange fixes... |
|
Sounds good. I'm not sure why the fuzzer didn't catch that -- perhaps it's likely to cover liveranges anyway by randomly using lots of values from earlier in the program... in any case, the logic looks correct now. |
2 participants
Branch parameters are not proper operands, so it doesn't make sense to represent them using
Operands. The constraint is irrelevant (you always wantAny), and so is the position within the instruction (early/late).This PR adds
Function::branch_paramswhich returns the branch parameters directly as a list ofVRegs.This is also useful for rematerialization (#8): branch parameters don't need to be in an actual register or stack slot if we can rematerialize them later. This would normally be required by the
Anyconstraint used for branch parameters.