[PATCH] D118455: [DebugInfo][InstrRef][NFC] Cache some PHI resolutions

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: StephenTozer, Orlando, TWeaver.
Herald added a subscriber: hiraditya.
jmorse requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Install a cache of DBG_INSTR_REF -> ValueIDNum resolutions, for scenarios where the value has to be reconstructed from several DBG_PHIs.  Whenever this happens, it's because branch folding + tail duplication has messed with the SSA form of the program, and we have to solve a mini SSA problem to find the variable value. This is always called twice, so it makes sense to cache the value.

There's a secondary reason, which is that the resolveDbgPHIs function makes use of the per-register machine-value tables, potentially late in the LiveDebugValues process. Those are about to be freed earlier in the next patch I upload, so it's good to cache and avoid the second call to resolveDbgPHIs. There's also a compile time improvement for a few benchmarks [0]

[0] http://llvm-compile-time-tracker.com/compare.php?from=7dfb73512c6ba1eb720e6a0da954f262c6bb53fe&to=fd6b2a1e76dc22f808f031e4e50b27a851776e48&stat=instructions


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118455

Files:
  llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
  llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h


Index: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
===================================================================
--- llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
+++ llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
@@ -867,6 +867,12 @@
   OverlapMap OverlapFragments;
   VarToFragments SeenFragments;
 
+  /// Mapping of DBG_INSTR_REF instructions to their values, for those
+  /// DBG_INSTR_REFs that call resolveDbgPHIs. These variable references solve
+  /// a mini SSA problem caused by DBG_PHIs being cloned, this colleciton caches
+  /// the result.
+  DenseMap<MachineInstr *, Optional<ValueIDNum>> SeenDbgPHIs;
+
   /// True if we need to examine call instructions for stack clobbers. We
   /// normally assume that they don't clobber SP, but stack probes on Windows
   /// do.
Index: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
===================================================================
--- llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -3092,6 +3092,7 @@
   DebugPHINumToValue.clear();
   OverlapFragments.clear();
   SeenFragments.clear();
+  SeenDbgPHIs.clear();
 
   return Changed;
 }
@@ -3357,6 +3358,12 @@
                                                       ValueIDNum **MLiveIns,
                                                       MachineInstr &Here,
                                                       uint64_t InstrNum) {
+  // This function will be called twice per DBG_INSTR_REF, and might end up
+  // computing lots of SSA information: memoize it.
+  auto SeenDbgPHIIt = SeenDbgPHIs.find(&Here);
+  if (SeenDbgPHIIt != SeenDbgPHIs.end())
+    return SeenDbgPHIIt->second;
+
   // Pick out records of DBG_PHI instructions that have been observed. If there
   // are none, then we cannot compute a value number.
   auto RangePair = std::equal_range(DebugPHINumToValue.begin(),
@@ -3406,6 +3413,7 @@
   if (AvailIt != AvailableValues.end()) {
     // Actually, we already know what the value is -- the Use is in the same
     // block as the Def.
+    SeenDbgPHIs.insert({&Here, ValueIDNum::fromU64(AvailIt->second)});
     return ValueIDNum::fromU64(AvailIt->second);
   }
 
@@ -3452,8 +3460,10 @@
     // Are all these things actually defined?
     for (auto &PHIIt : PHI->IncomingValues) {
       // Any undef input means DBG_PHIs didn't dominate the use point.
-      if (Updater.UndefMap.find(&PHIIt.first->BB) != Updater.UndefMap.end())
+      if (Updater.UndefMap.find(&PHIIt.first->BB) != Updater.UndefMap.end()) {
+        SeenDbgPHIs.insert({&Here, None});
         return None;
+      }
 
       ValueIDNum ValueToCheck;
       ValueIDNum *BlockLiveOuts = MLiveOuts[PHIIt.first->BB.getNumber()];
@@ -3471,8 +3481,10 @@
         ValueToCheck = VVal->second;
       }
 
-      if (BlockLiveOuts[Loc.asU64()] != ValueToCheck)
+      if (BlockLiveOuts[Loc.asU64()] != ValueToCheck) {
+        SeenDbgPHIs.insert({&Here, None});
         return None;
+      }
     }
 
     // Record this value as validated.
@@ -3481,5 +3493,6 @@
 
   // All the PHIs are valid: we can return what the SSAUpdater said our value
   // number was.
+  SeenDbgPHIs.insert({&Here, Result});
   return Result;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118455.403971.patch
Type: text/x-patch
Size: 3245 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220128/611ea2fb/attachment.bin>