[PATCH] D73510: Reland "[StackColoring] Remap PseudoSourceValue frame indices via MachineFunction::getPSVManager()""

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 27 15:20:08 PST 2020


MaskRay created this revision.
MaskRay added a reviewer: rnk.
Herald added subscribers: llvm-commits, jsji, hiraditya, nemanjai.
Herald added a project: LLVM.

Reland 7a8b0b1595e7dc878b48cf9bbaa652087a6895db <https://reviews.llvm.org/rG7a8b0b1595e7dc878b48cf9bbaa652087a6895db>, with a fix that checks
`!E.value().empty()` to avoid inserting a zero to SlotRemap.

Debugged by rnk@ in https://bugs.chromium.org/p/chromium/issues/detail?id=1045650#c33 . I don't know how to get a WinEH reproduce.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73510

Files:
  llvm/include/llvm/CodeGen/PseudoSourceValue.h
  llvm/lib/CodeGen/StackColoring.cpp
  llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir


Index: llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir
===================================================================
--- llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir
+++ llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir
@@ -1,4 +1,16 @@
-# RUN: llc -o - %s -start-before=stack-coloring
+# RUN: llc -run-pass=stack-coloring %s -o - | FileCheck %s
+
+## Test %stack.1 is merged into %stack.0 and there is no MemoryMemOperand
+## referencing %stack.1. This regression test is sensitive to the StackColoring
+## algorithm. Please adjust or delete this test if the merging strategy
+## changes.
+
+# CHECK:    {{^}}stack:
+# CHECK-NEXT: - { id: 0,
+# CHECK-NOT:  - { id: 1,
+# CHECK:      - { id: 2,
+# CHECK-NOT: %stack.1
+
 --- |
   ; ModuleID = '<stdin>'
   source_filename = "<stdin>"
Index: llvm/lib/CodeGen/StackColoring.cpp
===================================================================
--- llvm/lib/CodeGen/StackColoring.cpp
+++ llvm/lib/CodeGen/StackColoring.cpp
@@ -960,6 +960,7 @@
   }
 
   // Remap all instructions to the new stack slots.
+  std::vector<std::vector<MachineMemOperand *>> SSRefs(MFI->getObjectIndexEnd());
   for (MachineBasicBlock &BB : *MF)
     for (MachineInstr &I : BB) {
       // Skip lifetime markers. We'll remove them soon.
@@ -1025,13 +1026,14 @@
       SmallVector<MachineMemOperand *, 2> NewMMOs;
       bool ReplaceMemOps = false;
       for (MachineMemOperand *MMO : I.memoperands()) {
+        // Collect MachineMemOperands which reference
+        // FixedStackPseudoSourceValues with old frame indices.
         if (const auto *FSV = dyn_cast_or_null<FixedStackPseudoSourceValue>(
                 MMO->getPseudoValue())) {
           int FI = FSV->getFrameIndex();
           auto To = SlotRemap.find(FI);
           if (To != SlotRemap.end())
-            const_cast<FixedStackPseudoSourceValue *>(FSV)->setFrameIndex(
-                To->second);
+            SSRefs[FI].push_back(MMO);
         }
 
         // If this memory location can be a slot remapped here,
@@ -1071,6 +1073,15 @@
         I.setMemRefs(*MF, NewMMOs);
     }
 
+  // Rewrite MachineMemOperands that reference old frame indices.
+  for (auto E : enumerate(SSRefs))
+    if (!E.value().empty()) {
+      const PseudoSourceValue *NewSV =
+          MF->getPSVManager().getFixedStack(SlotRemap[E.index()]);
+      for (MachineMemOperand *Ref : E.value())
+        Ref->setValue(NewSV);
+    }
+
   // Update the location of C++ catch objects for the MSVC personality routine.
   if (WinEHFuncInfo *EHInfo = MF->getWinEHFuncInfo())
     for (WinEHTryBlockMapEntry &TBME : EHInfo->TryBlockMap)
Index: llvm/include/llvm/CodeGen/PseudoSourceValue.h
===================================================================
--- llvm/include/llvm/CodeGen/PseudoSourceValue.h
+++ llvm/include/llvm/CodeGen/PseudoSourceValue.h
@@ -93,7 +93,7 @@
 /// A specialized PseudoSourceValue for holding FixedStack values, which must
 /// include a frame index.
 class FixedStackPseudoSourceValue : public PseudoSourceValue {
-  int FI;
+  const int FI;
 
 public:
   explicit FixedStackPseudoSourceValue(int FI, const TargetInstrInfo &TII)
@@ -112,7 +112,6 @@
   void printCustom(raw_ostream &OS) const override;
 
   int getFrameIndex() const { return FI; }
-  void setFrameIndex(int FI) { this->FI = FI; }
 };
 
 class CallEntryPseudoSourceValue : public PseudoSourceValue {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73510.240696.patch
Type: text/x-patch
Size: 3393 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200127/ece0942a/attachment.bin>


More information about the llvm-commits mailing list