[PATCH] D94949: [AArch64][RegAllocFast] Add findSpillBefore to TargetRegisterInfo

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/AtomicLoopBundler.h:35
+private:
+  bool bundleBlock(MachineBasicBlock &MBB) {
+    // One of the basic blocks inserted by AtomicExpandPass looks like this:
----------------
I think running selection, and then trying to interpret the instructions to figure out what it did is fraught with peril. You would be better off expanding a pseudoinstruction after register allocation


================
Comment at: llvm/include/llvm/CodeGen/AtomicLoopBundler.h:43
+    //     br i1 %try_again, label %loop, label %atomicrmw.end
+    if (!MBB.getName().contains("atomicrmw.start"))
+      return false;
----------------
You absolutely cannot rely on machine basic block names


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1093
 
+  // If a bundle contains a virtual register def followed by a use of another,
+  // they must not be allocated the same physical register. The alternative
----------------
The order of instructions in the bundle doesn't actually matter. It's not accurate to say a def happens before or after a use inside the bundle


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1098
+  // bundle like an early-clobber.
+  const bool IsBundle = MI.getOpcode() == TargetOpcode::BUNDLE;
+
----------------
foad wrote:
> Naive question: shouldn't whatever added the operands to the BUNDLE have set the IsEarlyClobber flag appropriately, so you don't need to special-case bundles here?
Yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94949