[PATCH] D100296: [WIP] Teach Greedy RA to recognize special case live-through

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

I really really like this idea, but the implementation, not so much.  :)

I assume that you'd disable and eventually delete FixupCalleeSaved if this landed right?  (Mostly checking my understanding of the patch intent.)

Code structure wise, I think this needs to be merged into the existing loop.  If I'm reading the code correctly, you just need to add a special case when we'd otherwise exit the innermost loop.  If *SlotI == LiveI->end and Live->end is a statepoint deopt use, then add the regmask.  (I believe that SlotE is past the end of all possible instructions, and thus we can't have a statepoint there.)

You could also consider adjusting the existing code to use a lambda to do the lazy UsableRegs init.  That would be a lot easier to read even for the existing code, and your approach becomes easier to read if you're not duplicating all of the Found logic.

If my earlier comment didn't parse, I'd suggest extracting the helper as suggested inline and rebasing over the lambda.  I think once most of the extra complexity is out of the way, it'll be easier to see ways to fold the two loops together.



================
Comment at: llvm/lib/CodeGen/LiveIntervals.cpp:922
+    if (MachineInstr *MI = Indexes->getInstructionFromIndex(S.end))
+      if (MI->getOpcode() == TargetOpcode::STATEPOINT) {
+        StatepointOpers SO(MI);
----------------
I think you can make this code much more readable by adding a utility along the lines of the following:
bool hasLiveThroughUse(MachineInstr, Register);


================
Comment at: llvm/lib/CodeGen/LiveIntervals.cpp:930
+              // TODO - add reg mask
+              while (SlotI4SP != SlotE &&
+                     !SlotIndex::isSameInstr(*SlotI4SP, S.end))
----------------
Isn't this just a really weirdly spelled expression of getInstructionIndex on SlotIndexes?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100296/new/

https://reviews.llvm.org/D100296