[PATCH] D60237: [MS] Add metadata for __declspec(allocator)

[Amy Huang via Phabricator via cfe-commits]

akhuang marked an inline comment as done.
akhuang added inline comments.


================
Comment at: clang/lib/CodeGen/CGAtomic.cpp:1691
   } else {
-    // Build new lvalue for temp address
+    // Build new lvalue for temp address.
     Address Ptr = Atomics.materializeRValue(OldRVal);
----------------
rnk wrote:
> I don't have an issue with these changes if you want to make them, but they should be committed separately.
Whoops, these were supposed to be a separate test commit but I guess they ended up in here too. 


================
Comment at: clang/lib/CodeGen/CGCall.cpp:4384-4385
+    if (TargetDecl && TargetDecl->hasAttr<MSAllocatorAttr>())
+      CI->setMetadata("heapallocsite", getDebugInfo()->
+                       getMSAllocatorMetadata(RetTy, Loc));
+  }
----------------
rnk wrote:
> I think we should make CGDebugInfo responsible for calling setMetadata. In some sense, "heapallocsite" metadata is debug info, so it makes sense that it would be documented there. Also, if there are other places where we need to add this metadata, we won't have to duplicate this string literal.
> 
> So, CGDebugInfo should have some new method `addHeapAllocSiteMetadata(Instruction *CallSite, QualType Ty)`, and that can call the private getOrCreateType method. Sound good?
Makes sense.


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

https://reviews.llvm.org/D60237