[PATCH] D134098: [BOLT] Add pass to fix ambiguous memory references

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb added inline comments.


================
Comment at: bolt/lib/Passes/ValidateMemRefs.cpp:57-58
+void ValidateMemRefs::runOnFunction(BinaryFunction &BF) {
+  BinaryContext &BC = BF.getBinaryContext();
+  MCPlusBuilder *MIB = BC.MIB.get();
+
----------------



================
Comment at: bolt/lib/Passes/ValidateMemRefs.cpp:60
+
+  for (BinaryBasicBlock *BB : BF.getLayout().blocks()) {
+    for (MCInst &Inst : *BB) {
----------------
Is there a reason to iterate over layout (vs CFG)? If there's no reason, we should prefer CFG.


================
Comment at: bolt/lib/Passes/ValidateMemRefs.cpp:82
+  // Skip validation if not moving JT
+  if (opts::JumpTables != JTS_MOVE)
+    return;
----------------
Probably the condition needs to be modified as we are moving jump tables in other modes as well.


================
Comment at: bolt/lib/Passes/ValidateMemRefs.cpp:92
+  };
+  LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Starting memrefs validation pass\n");
+  ParallelUtilities::runOnEachFunctionWithUniqueAllocId(
----------------



================
Comment at: bolt/lib/Passes/ValidateMemRefs.cpp:94
+  ParallelUtilities::runOnEachFunctionWithUniqueAllocId(
+      BC, ParallelUtilities::SchedulingPolicy::SP_BB_QUADRATIC, ProcessFunction,
+      SkipPredicate, "validate-mem-refs", /*ForceSequential=*/true);
----------------
Is the complexity really quadratic?


================
Comment at: bolt/lib/Passes/ValidateMemRefs.cpp:102
+  outs() << "BOLT-INFO: validate-mem-refs updated " << ReplacedReferences
+         << " object references.\n";
+}
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134098