[PATCH] D137984: [BOLT] Fix state of MCSymbols in lowering pass

[Rafael Auler via Phabricator via llvm-commits]

rafauler updated this revision to Diff 476631.
rafauler added a comment.

Use a simpler strategy suggested by @maksfb, iterating over MCContext
symbol table.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137984

Files:
  bolt/include/bolt/Passes/BinaryPasses.h
  bolt/lib/Passes/BinaryPasses.cpp
  bolt/lib/Rewrite/BinaryPassManager.cpp


Index: bolt/lib/Rewrite/BinaryPassManager.cpp
===================================================================
--- bolt/lib/Rewrite/BinaryPassManager.cpp
+++ bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -479,6 +479,10 @@
 
   Manager.registerPass(std::make_unique<LowerAnnotations>(NeverPrint));
 
+  // Check for dirty state of MCSymbols caused by running calculateEmittedSize
+  // in parallel and restore them
+  Manager.registerPass(std::make_unique<CleanMCState>(NeverPrint));
+
   Manager.runPasses();
 }
 
Index: bolt/lib/Passes/BinaryPasses.cpp
===================================================================
--- bolt/lib/Passes/BinaryPasses.cpp
+++ bolt/lib/Passes/BinaryPasses.cpp
@@ -644,6 +644,30 @@
     BC.MIB->setOffset(*Item.first, Item.second);
 }
 
+// Check for dirty state in MCSymbol objects that might be a consequence
+// of running calculateEmittedSize() in parallel, during split functions
+// pass. If an inconsistent state is found (symbol already registered or
+// already defined), clean it.
+void CleanMCState::runOnFunctions(BinaryContext &BC) {
+  MCContext &Ctx = *BC.Ctx;
+  for (const auto &SymMapEntry : Ctx.getSymbols()) {
+    const MCSymbol *S = SymMapEntry.second;
+    if (S->isDefined()) {
+      LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Symbol \"" << S->getName()
+                        << "\" is already defined\n");
+      const_cast<MCSymbol *>(S)->setUndefined();
+    }
+    if (S->isRegistered()) {
+      LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Symbol \"" << S->getName()
+                        << "\" is already registered\n");
+      const_cast<MCSymbol *>(S)->setIsRegistered(false);
+    }
+    LLVM_DEBUG(if (S->isVariable()) {
+      dbgs() << "BOLT-DEBUG: Symbol \"" << S->getName() << "\" is variable\n";
+    });
+  }
+}
+
 namespace {
 
 // This peephole fixes jump instructions that jump to another basic
Index: bolt/include/bolt/Passes/BinaryPasses.h
===================================================================
--- bolt/include/bolt/Passes/BinaryPasses.h
+++ bolt/include/bolt/Passes/BinaryPasses.h
@@ -213,6 +213,16 @@
   void runOnFunctions(BinaryContext &BC) override;
 };
 
+/// Clean the state of the MC representation before sending it to emission
+class CleanMCState : public BinaryFunctionPass {
+public:
+  explicit CleanMCState(const cl::opt<bool> &PrintPass)
+      : BinaryFunctionPass(PrintPass) {}
+
+  const char *getName() const override { return "clean-mc-state"; }
+  void runOnFunctions(BinaryContext &BC) override;
+};
+
 /// An optimization to simplify conditional tail calls by removing
 /// unnecessary branches.
 ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137984.476631.patch
Type: text/x-patch
Size: 2606 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221119/9e9d8a47/attachment.bin>