[PATCH] D153990: [DebugInfo][RemoveDIs] Add prototype storage classes for non-instruction variable debug-info

Mon Oct 2 09:14:05 PDT 2023

jmorse added inline comments.

================
Comment at: llvm/include/llvm/IR/BasicBlock.h:65
+  /// block returns to a cannonical form when the terminator is re-inserted.
+  DPMarker TrailingDPValues;

----------------
Orlando wrote:
> How frequently do we expect TrailingDPValues to be used? Is it worth having this heap allocated instead to save some bits?
Yuh; in fact, given that this is a transitory state that crops up rarely, we can probably put a map in Function rather than making every block pay a penalty.

================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:2100-2102
+    MDNode *UniqueArgList = uniquify();
+    if (UniqueArgList != this) {
+      replaceAllUsesWith(UniqueArgList);
----------------
Orlando wrote:
> Please could you explain this change to me (why/what it's doing)? It seems the couple of months I've not touched metadata have clouded my memory.
I...... can't remember. Or at least, I think this comes from the work @StephenTozer did on these patches, and this deep into the metadata is out of my zone of comfort too. Any opinion on this Stephen?

I'll also see if we can just delete it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153990