hans added a comment. I don't really know about the functionality here, just adding a few comments on the code itself.
================ Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1966 + QualType PointeeTy = D.getTypePtr()->getPointeeType(); + llvm::DIType *DI = getOrCreateType(PointeeTy, getOrCreateFile(Loc)); + CI->setMetadata("heapallocsite", DI); ---------------- 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? ================ Comment at: clang/test/CodeGen/debug-info-codeview-heapallocsite.c:19 +// FIXME: Currently not testing that call_alloc_void has metadata because the +// metadata is not set when the type is void. + ---------------- Is it not set with your new code though? ================ Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1078 + MCSymbol *EndLabel = std::get<1>(HeapAllocSite); + DIType *DITy = cast<DIType>(std::get<2>(HeapAllocSite)); + ---------------- Is the cast necessary? Couldn't the tuple member be made a DIType* in the first place? ================ Comment at: llvm/test/CodeGen/X86/label-heapallocsite.ll:1 +; RUN: llc -O0 < %s | FileCheck %s +; FIXME: Add test for llc with optimizations once it is implemented. ---------------- 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? ================ Comment at: llvm/test/CodeGen/X86/label-heapallocsite.ll:16 +; ModuleID = 'heapallocsite.c' +source_filename = "heapallocsite.c" +target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128" ---------------- I guess the ModuleID and source_filename are unnecessary, so I'd dro pthem. ================ Comment at: llvm/test/CodeGen/X86/label-heapallocsite.ll:51 +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" } + ---------------- Both sets of attributes seem unnecessary so could probably be removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60800/new/ https://reviews.llvm.org/D60800 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits