[PATCH] D60800: [MS] Emit S_HEAPALLOCSITE debug info

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1966
+  QualType PointeeTy = D.getTypePtr()->getPointeeType();
+  llvm::DIType *DI = getOrCreateType(PointeeTy, getOrCreateFile(Loc));
+  CI->setMetadata("heapallocsite", DI);
----------------
akhuang wrote:
> hans wrote:
> > I don't really know this code, but does this also work for void pointers, i.e. the if statement in the old code was unnecessary?
> I think `getOrCreateType` returns null if it gets a void type, so it doesn't quite work for void pointers. In theory we shouldn't be getting void pointers here since the type should be cast to something but that hasn't been implemented yet.
I think in practice void will be pretty common, so I think we want to leave this as it is, and come up with some other workaround in CodeViewDebug.cpp. We can leave this as is and have the CodeViewDebug.cpp code interpret an empty tuple as a void type.

In hindsight, I think this "void is null" convention wasn't such a good idea, since we can't pass it through APIs like Instruction::setMetadata.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp:1239
+  if (CLI.CS && CLI.CS->getInstruction()->getMetadata("heapallocsite")) {
+    DIType *DI = cast<DIType>(CLI.CS->getInstruction()->
+                              getMetadata("heapallocsite"));
----------------
I think you could make `addCodeViewHeapAllocSite` take an `MDNode*` and then do this cast inside, and if the cast fails, see if it's the special empty tuple that means void, and put a null `DIType*` in the pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60800