[PATCH] D70286: Merge memtag instructions with adjacent stack slots.

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2718
+  // Add an instruction to be replaced. Instructions must be added in the
+  // ascending order of Offset, and have to be adjacent.
+  void AddInstruction(TagStoreInstr I) { TagStores.push_back(I); }
----------------
> Instructions must be added in the ascending order of Offset, and have to be adjacent.

Could we add an assertion to verify this? Either TagStores must be empty, or this instruction must be adjacent to the last one.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2854
+    MachineInstr *MI = TS.MI;
+    if (MI->memoperands_empty()) {
+      MemRefs.clear();
----------------
Why do we want to return an empty list if any one input instruction doesn't have a memoperand?


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2882
+
+  const int kSetTagLoopThreshold = 176;
+  if (Size < kSetTagLoopThreshold) {
----------------
Where does the number 176 come from?


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:3012
+  // New code will be inserted after the last tagging instruction we've found.
+  MachineBasicBlock::iterator InsertI = Instrs[Instrs.size() - 1].MI;
+  InsertI++;
----------------
This can be `Instrs.back()`


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:1517
 
 // Large STG to be expanded into a loop. $Rm is the size, $Rn is start address.
 // $Rn_wback is one past the end of the range.
----------------
This comment needs updating to mention the size operand.


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.h:127
 };
-
 } // end namespace llvm
----------------
Whitespace change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70286