[PATCH] D137574: PEI should be able to use backward walk in replaceFrameIndicesBackward.

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 10 13:32:59 PST 2022


arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetRegisterInfo.h:1031-1034
+  /// Should be overriden by target if it is ready to
+  /// use backward register scavenger walk.
+  /// Currently is used in PEI::replaceFrameIndices to call the
+  /// replaceFrameIndicesBackward or use the old method.
----------------
This comment isn't particularly enlightening. How about



Last time I tried this, I was trying to change all the target implementations to change the signature to make the reverse iteration less ugly. Probably shouldn't let this remain for long and just update all the targets


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1458
+  
+  for (MachineInstr &MI : llvm::make_early_inc_range(reverse(*BB))) {
+    
----------------
Don't need llvm::


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1505-1506
+    // Shift it to make RS collect reg info up to the current instruction.
+    if (Step != BB->begin())
+      Step--;
+
----------------
Do we have test coverage for a frame index that needs to be handled as the very first and very last instruction in the block? If not, we need them 


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1448
+  
+  for (MachineInstr &MI : llvm::make_early_inc_range(reverse(*BB))) {
+    
----------------
Don't need llvm::


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137574



More information about the llvm-commits mailing list