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

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added a comment.

Seems like a good change - the compile time improvement is nice, and this doesn't look like it should have a noticeable memory footprint. LGTM with the other comments addressed.



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:3416-3417
     // block as the Def.
+    SeenDbgPHIs.insert({&Here, ValueIDNum::fromU64(AvailIt->second)});
     return ValueIDNum::fromU64(AvailIt->second);
   }
----------------
dblaikie wrote:
> Given the need to touch every return from this function, and they're fairly spread out - might be good to refactor the non-cache-hit part of the function into another function, call that from the caching version and only have to have one `SeenDbgPHIs.insert` call that then ends up relatively close to the `SeenDbgPHIs` lookup code too?
> 
> (& I guess operations in this function invalidate the `SeenDbgPHIs` iterator, otherwise you could use that (possibly even call `insert`) later on to save the extra lookup)
+1, better readability + prevents someone in the future from adding a new return without the cache insert.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h:872
+  /// 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.
----------------
Nit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118455