[MS] Emit S_HEAPALLOCSITE debug info

[akhuang]

Summary

This emits labels around heapallocsite calls and S_HEAPALLOCSITE debug
info in codeview. Currently only changes FastISel, so emitting labels still
needs to be implemented in SelectionDAG.

[akhuang]

remove comment

[akhuang]

more typos

[akhuang]

Fix test case

[hans]

I don't really know about the functionality here, just adding a few comments on the code itself.

clang/lib/CodeGen/CGDebugInfo.cpp

1966

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?

clang/test/CodeGen/debug-info-codeview-heapallocsite.c

19

Is it not set with your new code though?

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

1078

Is the cast necessary? Couldn't the tuple member be made a DIType* in the first place?

llvm/test/CodeGen/X86/label-heapallocsite.ll

1

Does llc have a "-fast-isel" flag or similar that could be used instead, to make it more clear that it's fast-isel that's significant for the test?

16

I guess the ModuleID and source_filename are unnecessary, so I'd dro pthem.

51

Both sets of attributes seem unnecessary so could probably be removed.

[akhuang]

Removed extraneous information from test; changed type to DIType

[akhuang]

clang/lib/CodeGen/CGDebugInfo.cpp

1966

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.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

1078

Changed the tuple member to be a DIType.

[rnk]

clang/lib/CodeGen/CGDebugInfo.cpp

1966

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.

llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

1239

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.

[akhuang]

• Pass void metadata as null DIType

[akhuang]

• Changed test case back to original

[akhuang]

llvm/test/CodeGen/X86/label-heapallocsite.ll

1

I couldn't find a flag that makes llc use fast-isel; it should soon work for both cases though.

[rnk]

lgtm with a minor whitespace issue

clang/lib/CodeGen/CGDebugInfo.cpp

1972

Please revert the white space only change to this file.

llvm/lib/CodeGen/MachineFunction.cpp

815

I guess it's reasonable to treat any non-DIType as "void". The only other reasonable thing to do would be to report an error, but it's not worth it.

llvm/test/CodeGen/X86/label-heapallocsite.ll

1

FWIW, -O0 is the typical way to enable fast isel in other codegen tests, so even if it's opaque, it's consistent.

[akhuang]

whitespace fix

clang/lib/CodeGen/CGDebugInfo.cpp

llvm::DIType *CGDebugInfo::getOrCreateStandaloneType(QualType D,
 
   RetainedTypes.push_back(D.getAsOpaquePtr());
   return T;
 }
 
 void CGDebugInfo::addHeapAllocSiteMetadata(llvm::Instruction *CI,
                                            QualType D,
                                            SourceLocation Loc) {
-  llvm::MDNode *node;
-  if (D.getTypePtr()->isVoidPointerType()) {
-    node = llvm::MDNode::get(CGM.getLLVMContext(), None);
-  } else {
-    QualType PointeeTy = D.getTypePtr()->getPointeeType();
-    node = getOrCreateType(PointeeTy, getOrCreateFile(Loc));
-  }
-
-  CI->setMetadata("heapallocsite", node);
+  llvm::DIType *DI = getOrCreateType(PointeeTy, getOrCreateFile(Loc));
+  CI->setMetadata("heapallocsite", DI);
 }
 
 void CGDebugInfo::completeType(const EnumDecl *ED) {
   if (DebugKind <= codegenoptions::DebugLineTablesOnly)
     return;
   QualType Ty = CGM.getContext().getEnumType(ED);
   void *TyPtr = Ty.getAsOpaquePtr();
   auto I = TypeCache.find(TyPtr);

clang/test/CodeGen/debug-info-codeview-heapallocsite.c

 void call_alloc_void() {
   struct Foo *p = (struct Foo*)alloc_void();
 }
 
 void call_alloc_foo() {
   struct Foo *p = alloc_foo();
 }
 
+// FIXME: Currently not testing that call_alloc_void has metadata because the
+// metadata is not set when the type is void.
+
 // CHECK-LABEL: define {{.*}}void @call_alloc_void
-// CHECK: call i8* {{.*}}@alloc_void{{.*}} !heapallocsite [[DBG1:!.*]]
 
 // CHECK-LABEL: define {{.*}}void @call_alloc_foo
 // CHECK: call %struct.Foo* {{.*}}@alloc_foo{{.*}} !heapallocsite [[DBG2:!.*]]
 
-// CHECK: [[DBG1]] = !{}
 // CHECK: [[DBG2]] = distinct !DICompositeType(tag: DW_TAG_structure_type,
 // CHECK-SAME:                                 name: "Foo"
 

llvm/include/llvm/CodeGen/MachineFunction.h

class MachineFunction {
   DenseMap<const MachineBasicBlock *, unsigned> WasmLPadToIndexMap;
 
   /// Map of invoke call site index values to associated begin EH_LABEL.
   DenseMap<MCSymbol*, unsigned> CallSiteMap;
 
   /// CodeView label annotations.
   std::vector<std::pair<MCSymbol *, MDNode *>> CodeViewAnnotations;
 
+  /// CodeView heapallocsites.
+  std::vector<std::tuple<MCSymbol*, MCSymbol*, MDNode*>>
+      CodeViewHeapAllocSites;
+
   bool CallsEHReturn = false;
   bool CallsUnwindInit = false;
   bool HasEHScopes = false;
   bool HasEHFunclets = false;
 
   /// List of C++ TypeInfo used.
   std::vector<const GlobalValue *> TypeInfos;
 
public:
   void addCodeViewAnnotation(MCSymbol *Label, MDNode *MD) {
     CodeViewAnnotations.push_back({Label, MD});
   }
 
   ArrayRef<std::pair<MCSymbol *, MDNode *>> getCodeViewAnnotations() const {
     return CodeViewAnnotations;
   }
 
+  /// Record heapallocsites
+  void addCodeViewHeapAllocSite(MachineInstr *I, MDNode *MD);
+
+  ArrayRef<std::tuple<MCSymbol*, MCSymbol*, MDNode*>>
+      getCodeViewHeapAllocSites() const {
+    return CodeViewHeapAllocSites;
+  }
+
   /// Return a reference to the C++ typeinfo for the current function.
   const std::vector<const GlobalValue *> &getTypeInfos() const {
     return TypeInfos;
   }
 
   /// Return a reference to the typeids encoding filters used in the current
   /// function.
   const std::vector<unsigned> &getFilterIds() const {

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

struct FunctionInfo {
     SmallVector<CVGlobalVariable, 1> Globals;
 
     std::unordered_map<const DILexicalBlockBase*, LexicalBlock> LexicalBlocks;
 
     // Lexical blocks containing local variables.
     SmallVector<LexicalBlock *, 1> ChildBlocks;
 
     std::vector<std::pair<MCSymbol *, MDNode *>> Annotations;
+    std::vector<std::tuple<MCSymbol *, MCSymbol *, MDNode *>> HeapAllocSites;
 
     const MCSymbol *Begin = nullptr;
     const MCSymbol *End = nullptr;
     unsigned FuncId = 0;
     unsigned LastFileId = 0;
 
     /// Number of bytes allocated in the prologue for all local stack objects.
     unsigned FrameSize = 0;

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

for (auto Annot : FI.Annotations) {
         // nice .asciz directive.
         StringRef Str = cast<MDString>(MD)->getString();
         assert(Str.data()[Str.size()] == '\0' && "non-nullterminated MDString");
         OS.EmitBytes(StringRef(Str.data(), Str.size() + 1));
       }
       endSymbolRecord(AnnotEnd);
     }
 
+    for (auto HeapAllocSite : FI.HeapAllocSites) {
+      MCSymbol *BeginLabel = std::get<0>(HeapAllocSite);
+      MCSymbol *EndLabel = std::get<1>(HeapAllocSite);
+      DIType *DITy = cast<DIType>(std::get<2>(HeapAllocSite));
+
+      MCSymbol *HeapAllocEnd = beginSymbolRecord(SymbolKind::S_HEAPALLOCSITE);
+      OS.AddComment("Call site offset");
+      OS.EmitCOFFSecRel32(BeginLabel, /*Offset=*/0);
+      OS.AddComment("Call site section index");
+      OS.EmitCOFFSectionIndex(BeginLabel);
+      OS.AddComment("Call instruction length");
+      OS.emitAbsoluteSymbolDiff(EndLabel, BeginLabel, 2);
+      OS.AddComment("Type index");
+      OS.EmitIntValue(getCompleteTypeIndex(DITy).getIndex(), 4);
+      endSymbolRecord(HeapAllocEnd);
+    }
+
     if (SP != nullptr)
       emitDebugInfoForUDTs(LocalUDTs);
 
     // We're done with this function.
     emitEndSymbolRecord(SymbolKind::S_PROC_ID_END);
   }
   endCVSubsection(SymbolsEnd);
 
void CodeViewDebug::endFunctionImpl(const MachineFunction *MF) {
   // Thunks are compiler-generated and probably won't have source correlation.
   if (!CurFn->HaveLineInfo && !GV.getSubprogram()->isThunk()) {
     FnDebugInfo.erase(&GV);
     CurFn = nullptr;
     return;
   }
 
   CurFn->Annotations = MF->getCodeViewAnnotations();
+  CurFn->HeapAllocSites = MF->getCodeViewHeapAllocSites();
 
   CurFn->End = Asm->getFunctionEnd();
 
   CurFn = nullptr;
 }
 
 void CodeViewDebug::beginInstruction(const MachineInstr *MI) {
   DebugHandlerBase::beginInstruction(MI);

llvm/lib/CodeGen/MachineFunction.cpp

try_next:;
   int FilterID = -(1 + FilterIds.size());
   FilterIds.reserve(FilterIds.size() + TyIds.size() + 1);
   FilterIds.insert(FilterIds.end(), TyIds.begin(), TyIds.end());
   FilterEnds.push_back(FilterIds.size());
   FilterIds.push_back(0); // terminator
   return FilterID;
 }
 
+void MachineFunction::addCodeViewHeapAllocSite(MachineInstr *I, MDNode *MD) {
+  MCSymbol *BeginLabel = Ctx.createTempSymbol("heapallocsite", true);
+  MCSymbol *EndLabel = Ctx.createTempSymbol("heapallocsite", true);
+  I->setPreInstrSymbol(*this, BeginLabel);
+  I->setPostInstrSymbol(*this, EndLabel);
+  CodeViewHeapAllocSites.push_back({BeginLabel, EndLabel, MD});
+}
+
 /// \}
 
 //===----------------------------------------------------------------------===//
 //  MachineJumpTableInfo implementation
 //===----------------------------------------------------------------------===//
 
 /// Return the size of each entry in the jump table.
 unsigned MachineJumpTableInfo::getEntrySize(const DataLayout &TD) const {

llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

bool FastISel::lowerCallTo(CallLoweringInfo &CLI) {
 
   // Set all unused physreg defs as dead.
   assert(CLI.Call && "No call instruction specified.");
   CLI.Call->setPhysRegsDeadExcept(CLI.InRegs, TRI);
 
   if (CLI.NumResultRegs && CLI.CS)
     updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs);
 
+  // Set labels for heapallocsite call.
+  if (CLI.CS && CLI.CS->getInstruction()->getMetadata("heapallocsite"))
+    MF->addCodeViewHeapAllocSite(CLI.Call, CLI.CS->getInstruction()->
+                                 getMetadata("heapallocsite"));
+
   return true;
 }
 
 bool FastISel::lowerCall(const CallInst *CI) {
   ImmutableCallSite CS(CI);
 
   FunctionType *FuncTy = CS.getFunctionType();
   Type *RetTy = CS.getType();

llvm/test/CodeGen/X86/label-heapallocsite.ll

+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;      -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc();
+; void g();
+; void foo() {
+;   g();
+;   myalloc()
+;   g();
+; }
+
+; ModuleID = 'heapallocsite.c'
+source_filename = "heapallocsite.c"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+; Function Attrs: noinline nounwind optnone
+define dso_local void @f() #0 !dbg !7 {
+entry:
+  call void @g(), !dbg !11
+  %call = call i8* @myalloc(), !dbg !12, !heapallocsite !13
+  call void @g(), !dbg !14
+  ret void, !dbg !15
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: callq g
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423                    # Record kind: S_GPROC32_ID
+; CHECK:       .short  4446                    # Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32       .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short  .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 112
+; CHECK-NEXT:  .p2align 2
+
+; CHECK-LABEL: .short  4431                    # Record kind: S_PROC_ID_END
+
+declare dso_local void @g() #1
+
+declare dso_local i8* @myalloc() #1
+
+attributes #0 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "<stdin>", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 4, type: !9, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "heapallocsite.c", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 5, scope: !7)
+!12 = !DILocation(line: 6, scope: !7)
+!13 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!14 = !DILocation(line: 7, scope: !7)
+!15 = !DILocation(line: 8, scope: !7)
+