[PATCH] D71599: [LLVM] Support to emit extern variables debuginfo with "-fstandalone-debug"

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 21 10:00:16 PST 2020


vsk added a comment.

Please fill out the 'Summary' field in the Phab review so the context is easier to pick up for reviewers.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:172
+    if (DD->useAllLinkageNames())
+    addLinkageName(*VariableDIE, GV->getLinkageName());
+  }
----------------
Why is a linkage name needed on a global variable declaration? Could you leave a comment that explains this?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:173
+    addLinkageName(*VariableDIE, GV->getLinkageName());
+  }
   else
----------------
Please clang-format your diffs so that they conform to llvm's coding standards (https://www.llvm.org/docs/CodingStandards.html#source-code-formatting)


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:186
+  if (GV->isDefinition())
   addLocationAttribute(VariableDIE, GV, GlobalExprs);
 
----------------
How is this source change covered by your test? If not, could you please add test coverage?


================
Comment at: llvm/test/DebugInfo/Generic/debug-info-extern-variable-declaration.ll:31
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "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-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
----------------
Could you reduce this test case by deleting unnecessary attributes?


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

https://reviews.llvm.org/D71599





More information about the llvm-commits mailing list