Multi-phase drreg use hides app value from later phases #5121
Comments
|
This seems like a significant problem: not the drreg_get_app_value(), which is rare, but the automated restore before app reads, which is quite common and easily corrupts and crashes the app if not done correctly every time. Does x86 scatter/gather expansion never reserve a register that's used in its own expansion, and that's what makes this problem not occur today? If so, is that just because the expansion only uses a small set of registers and so there are enough unused ones available, even in 32-bit? |
Expanding on this: even if I reserve a reg in app2app across several instructions and I carefully restore the app value manually before app instrs that read that register, that's not enough. The insertion phase could later use that same register, but have the wrong value stored in its slot, and think it's restoring the app value as part of its automated pass before that app read but instead it's undoing my manual restore and it's writing my tool value into that register. |
Note that the insertion phase will treat the app2app restore as an app write, and update the reg's value in its slot. So the insertion phase slot will contain the actual app value after the app2app restore. Correct drreg usage in app2app phase requires: manually restoring the app value before app reads, and manually re-spilling app value after app writes. My understanding is that if these are done correctly, then there shouldn't be any issue (note that incorrect app2app usage is a possibility even without multi-phase use). If you have a case in mind, an example would help. |
Adds a new drreg test to show another multi-phase use scenario. This demonstrates that the insertion phase respills the original app value to its slot after the app2app phase restores it. This results in overwriting the app2app meta value that was present in the insertion phase slot previously. This original app value is then restored by insertion phase before subsequent app read instrs. Issue: #5121
|
I added a new test in drreg-test to demonstrate this: PR #5124. Is this the case you were thinking of? |
Adds a test that demonstrates how to get the app value for a reg that was reserved in multiple phases using drreg. This is complex as in the current design a phase doesn't know about spill done in earlier phases. So, getting the app value in insertion phase for a reg that was reserved in app2app and insertion phases both requires both the phases to work together. Adds to the documentation of drreg_get_app_value and drreg_statelessly_restore_app_value that they allow obtaining the reg's meta value from the last phase where it was reserved, OR the app value if there's no such previous phase. Issue: #5121
This would only happen if drreg treated both app and tool instructions from app2app the same (as app instructions): yes, this does seem to be the case. But given that: there is no significant new issue raised with treating an annotation label as an app read, and so no concern with Proposal A from #5118 (comment), unlike in the original discussion: whoever is doing restores before app reads in app2app needs to do the same before an annotation label is all. |
Yes, something like PR #5126. Thoughts on submitting that PR so that we can point to that example for how app2app and insertion phases need to work together? |
…5124) Adds a new drreg test to show another multi-phase use scenario. This demonstrates that the insertion phase updates the reg value in its own slot after the app2app phase restores the reg to the original app value before an app read. This results in overwriting the app2app meta value that was present in the insertion phase's slot previously. This original app value is then restored correctly by insertion phase before subsequent app read instrs. Instrumented basic block for the new test#38, after app2app and insertion phase (each drreg op is commented): https://gist.github.com/abhinav92003/6a663cb19fda01838451bbe41b329bdc Issue: #5121
Today, DR supports multi-phase use for drreg, which means that clients can use drreg to get scratch regs in insertion phase and other phases as well.
#3823 added support to avoid spill slot conflicts in multi-phase use. However, different phases can still get the same scratch reg. In such "nested" spill regions,
drreg_get_app_valuewon't work as expected in later phases, as it will give the value of the reg in the earlier phase, not the app value. This should be fixed, and documented until then.Another related note:
Drreg support in non-insertion phases is bare bones -- e.g. it does not automatically restore app value before app read, or re-spill app value after app writes. This is usually fine because non-insertion phase is pretty limited. Today, we do that to expand scatter/gather instructions in app2app phase. But we should document this restriction about non-insertion phase use.
The text was updated successfully, but these errors were encountered: