Hi David,
Thanks for reviewing my patch. I try to address your concerns below.
> lib/CodeGen/CGDebugInfo.cpp#3058
> This comment seems suspiciously like we're working around something in a
> not-great way, but I don't understand: why are we setting the location at
> all here, do you know or did you copy this from elsewhere?
I do not know either. In the original codes, EmitGlobalVariable() for
non-deferred global variables sets source locations; the version for deferred
global variables does not set source location. So when I combine these two
versions in this patch, I added a condition check to preserve the original
code semantics. I tried to disable this particular line of codes, and the
test results in LLVM/Clang regression suites were still clean...
But on the other hand, I think this is not really relevant to this bug, and
if we decide to remove this line, it should be done in a separate patch. Is
it okay to add a FIXME comment there for now?
> test/CodeGen/debug-info-line5.cpp#2
> Clang tests should only test the generated IR without invoking the LLVM
> backend to emit object files, etc. If your change is purely in Clang, the
> should be possible to test it by only exercising Clang.
Thank you for pointing it out. Fixed.
> test/CodeGen/debug-info-scope.c#28
> Any reason you needed to add this 'foo' function as well as referencing
> 'd' at the end of the previous function?
Not really. I removed this local function.
> test/CodeGenCXX/debug-info-namespace.cpp#39
> Thanks for putting all these test cases in reasonable existing files.
> Though please try to be consistent with existing formatting in files (you
> could continue to use single letter class names "C", "D", etc rather than
> monkey/ape - simple, metasyntactic variables can be easy to follow rather
> than adding extra unnecessary semantics to the test cases) and usually with
> the LLVM style guide (so, '{' at the end of the line, rather than the
> beginning)
Fixed (hopefully).
http://llvm-reviews.chandlerc.com/D1281
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1281?vs=3583&id=3907#toc
Files:
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CGDebugInfo.h
lib/CodeGen/CGExpr.cpp
test/CodeGen/debug-info-scope.c
test/CodeGen/debug-info-line5.h
test/CodeGen/debug-info-line5.cpp
test/CodeGenCXX/debug-info-namespace.cpp
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -1379,12 +1379,27 @@
return EmitLValue(E).getAddress();
}
-void CodeGenFunction::EmitDeclRefExprDbgValue(const DeclRefExpr *E,
- llvm::Constant *Init) {
+void CodeGenFunction::EmitValueDeclDbgValue(const ValueDecl *Val,
+ llvm::Constant *Init) {
assert (Init && "Invalid DeclRefExpr initializer!");
- if (CGDebugInfo *Dbg = getDebugInfo())
- if (CGM.getCodeGenOpts().getDebugInfo() >= CodeGenOptions::LimitedDebugInfo)
- Dbg->EmitGlobalVariable(E->getDecl(), Init);
+ CGDebugInfo *Dbg = getDebugInfo();
+ if (!Dbg ||
+ CGM.getCodeGenOpts().getDebugInfo() < CodeGenOptions::LimitedDebugInfo)
+ return;
+
+ // Make sure we emit a debug reference to the global variable.
+ if (const VarDecl *VD = dyn_cast<VarDecl>(Val)) {
+ // Do not duplicate DIE entry for local variables; they are not deferred
+ // like global variables are.
+ if (VD->isFileVarDecl() && !getLangOpts().EmitAllDecls &&
+ !getContext().DeclMustBeEmitted(Val))
+ Dbg->EmitGlobalVariable(Init, VD);
+
+ // Make sure we emit a debug reference to an enumerator constant.
+ } else {
+ assert(isa<EnumConstantDecl>(Val));
+ Dbg->EmitEnumConstant(dyn_cast<EnumConstantDecl>(Val));
+ }
}
CodeGenFunction::PeepholeProtection
Index: lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3041,14 +3041,31 @@
}
/// EmitGlobalVariable - Emit information about a global variable.
-void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,
+/// \param VarOrInit either the global variable itself or the initializer
+/// \param D the global declaration
+void CGDebugInfo::EmitGlobalVariable(llvm::Value *VarOrInit,
const VarDecl *D) {
assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
// Create global variable debug descriptor.
llvm::DIFile Unit = getOrCreateFile(D->getLocation());
unsigned LineNo = getLineNumber(D->getLocation());
+ StringRef DeclName = D->getName();
+ StringRef LinkageName;
+ bool IsLocalToUnit = true;
- setLocation(D->getLocation());
+ // For deferred global variables, the current source location is usually
+ // where they are being referenced. Do not change the current source location
+ // to the place where they are declared, lest we get a bogus line table.
+ // FIXME: maybe we do not need to set the source location here at all.
+ if (llvm::GlobalVariable *Var = dyn_cast<llvm::GlobalVariable>(VarOrInit)) {
+ setLocation(D->getLocation());
+ IsLocalToUnit = Var->hasInternalLinkage();
+ if (D->getDeclContext() && !isa<FunctionDecl>(D->getDeclContext())
+ && !isa<ObjCMethodDecl>(D->getDeclContext()))
+ LinkageName = Var->getName();
+ if (LinkageName == DeclName)
+ LinkageName = StringRef();
+ }
QualType T = D->getType();
if (T->isIncompleteArrayType()) {
@@ -3060,19 +3077,12 @@
T = CGM.getContext().getConstantArrayType(ET, ConstVal,
ArrayType::Normal, 0);
}
- StringRef DeclName = D->getName();
- StringRef LinkageName;
- if (D->getDeclContext() && !isa<FunctionDecl>(D->getDeclContext())
- && !isa<ObjCMethodDecl>(D->getDeclContext()))
- LinkageName = Var->getName();
- if (LinkageName == DeclName)
- LinkageName = StringRef();
llvm::DIDescriptor DContext =
getContextDescriptor(dyn_cast<Decl>(D->getDeclContext()));
llvm::DIGlobalVariable GV =
DBuilder.createStaticVariable(DContext, DeclName, LinkageName, Unit,
LineNo, getOrCreateType(T, Unit),
- Var->hasInternalLinkage(), Var,
+ IsLocalToUnit, VarOrInit,
getStaticDataMemberDeclaration(D));
DeclCache.insert(std::make_pair(D->getCanonicalDecl(), llvm::WeakVH(GV)));
}
@@ -3103,26 +3113,16 @@
Var->hasInternalLinkage(), Var);
}
-/// EmitGlobalVariable - Emit global variable's debug info.
-void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD,
- llvm::Constant *Init) {
+/// EmitEnumConstant - Emit debug info for an enumerator constant
+void CGDebugInfo::EmitEnumConstant(const EnumConstantDecl *ECD)
+{
assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
- // Create the descriptor for the variable.
- llvm::DIFile Unit = getOrCreateFile(VD->getLocation());
- StringRef Name = VD->getName();
- llvm::DIType Ty = getOrCreateType(VD->getType(), Unit);
- if (const EnumConstantDecl *ECD = dyn_cast<EnumConstantDecl>(VD)) {
- const EnumDecl *ED = cast<EnumDecl>(ECD->getDeclContext());
- assert(isa<EnumType>(ED->getTypeForDecl()) && "Enum without EnumType?");
- Ty = getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
- }
- // Do not use DIGlobalVariable for enums.
- if (Ty.getTag() == llvm::dwarf::DW_TAG_enumeration_type)
- return;
- llvm::DIGlobalVariable GV = DBuilder.createStaticVariable(
- Unit, Name, Name, Unit, getLineNumber(VD->getLocation()), Ty, true, Init,
- getStaticDataMemberDeclaration(cast<VarDecl>(VD)));
- DeclCache.insert(std::make_pair(VD->getCanonicalDecl(), llvm::WeakVH(GV)));
+ llvm::DIFile Unit = getOrCreateFile(ECD->getLocation());
+ llvm::DIType Ty = getOrCreateType(ECD->getType(), Unit);
+
+ const EnumDecl *ED = cast<EnumDecl>(ECD->getDeclContext());
+ assert(isa<EnumType>(ED->getTypeForDecl()) && "Enum without EnumType?");
+ Ty = getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
}
llvm::DIScope CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2052,8 +2052,9 @@
LValue EmitStmtExprLValue(const StmtExpr *E);
LValue EmitPointerToDataMemberBinaryExpr(const BinaryOperator *E);
LValue EmitObjCSelectorLValue(const ObjCSelectorExpr *E);
- void EmitDeclRefExprDbgValue(const DeclRefExpr *E, llvm::Constant *Init);
+ void EmitValueDeclDbgValue(const ValueDecl *Val, llvm::Constant *Init);
+
//===--------------------------------------------------------------------===//
// Scalar Expression Emission
//===--------------------------------------------------------------------===//
Index: lib/CodeGen/CGDebugInfo.h
===================================================================
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -264,13 +264,13 @@
CGBuilderTy &Builder);
/// EmitGlobalVariable - Emit information about a global variable.
- void EmitGlobalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
+ void EmitGlobalVariable(llvm::Value *VarOrInit, const VarDecl *Decl);
/// EmitGlobalVariable - Emit information about an objective-c interface.
void EmitGlobalVariable(llvm::GlobalVariable *GV, ObjCInterfaceDecl *Decl);
- /// EmitGlobalVariable - Emit global variable's debug info.
- void EmitGlobalVariable(const ValueDecl *VD, llvm::Constant *Init);
+ /// EmitEnumConstant - Emit information about an enumerator constant
+ void EmitEnumConstant(const EnumConstantDecl *ECD);
/// \brief - Emit C++ using directive.
void EmitUsingDirective(const UsingDirectiveDecl &UD);
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -966,15 +966,9 @@
// Emit as a constant.
llvm::Constant *C = CGM.EmitConstantValue(result.Val, resultType, this);
- // Make sure we emit a debug reference to the global variable.
- // This should probably fire even for
- if (isa<VarDecl>(value)) {
- if (!getContext().DeclMustBeEmitted(cast<VarDecl>(value)))
- EmitDeclRefExprDbgValue(refExpr, C);
- } else {
- assert(isa<EnumConstantDecl>(value));
- EmitDeclRefExprDbgValue(refExpr, C);
- }
+ // Make sure we emit a debug reference to the global variable or
+ // enumerator constant.
+ EmitValueDeclDbgValue(value, C);
// If we emitted a reference constant, we need to dereference that.
if (resultIsReference)
Index: test/CodeGen/debug-info-scope.c
===================================================================
--- test/CodeGen/debug-info-scope.c
+++ test/CodeGen/debug-info-scope.c
@@ -1,7 +1,16 @@
// RUN: %clang_cc1 -g -emit-llvm < %s | FileCheck %s
+
+// Make sure that the debug info of the local variable d does not shadow
+// the global variable d
+// CHECK: [ DW_TAG_variable ] [d] [line 6] [def]
+const int d = 100;
+
// Two variables with same name in separate scope.
// Radar 8330217.
int main() {
+// CHECK-NOT: [ DW_TAG_variable ] [d] [line 13]
+// CHECK: [ DW_TAG_auto_variable ] [d] [line 13]
+ const int d = 4;
int j = 0;
int k = 0;
// CHECK: DW_TAG_auto_variable ] [i]
@@ -12,5 +21,5 @@
// CHECK-NEXT: DW_TAG_lexical_block
for (int i = 0; i < 10; i++)
k++;
- return 0;
+ return d; // make a reference to d so that its debug info gets included
}
Index: test/CodeGen/debug-info-line5.h
===================================================================
--- test/CodeGen/debug-info-line5.h
+++ test/CodeGen/debug-info-line5.h
@@ -0,0 +1,2 @@
+// Declaration of "Marker"
+const unsigned Marker = 0xFF999999;
Index: test/CodeGen/debug-info-line5.cpp
===================================================================
--- test/CodeGen/debug-info-line5.cpp
+++ test/CodeGen/debug-info-line5.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang %s -g -gcolumn-info -S -emit-llvm -o - | FileCheck %s
+// Line table entries should reference this cpp file, not the header
+
+#include "debug-info-line5.h"
+
+int result;
+int foo(unsigned);
+
+int main()
+{
+ while ( 1 )
+ {
+ result = foo(Marker);
+ }
+ return result;
+}
+
+// CHECK: !{{[0-9]*}} = metadata !{i32 {{[0-9]*}}, i32 {{[0-9]*}}, null, metadata !"Marker", {{.*}} ; [ DW_TAG_variable ] [Marker]
Index: test/CodeGenCXX/debug-info-namespace.cpp
===================================================================
--- test/CodeGenCXX/debug-info-namespace.cpp
+++ test/CodeGenCXX/debug-info-namespace.cpp
@@ -39,6 +39,16 @@
// This should work even if 'i' and 'func' were declarations & not definitions,
// but it doesn't yet.
+namespace C
+{
+ const int j = 32;
+}
+
+int func2()
+{
+ return C::j;
+}
+
// CHECK: [[CU:![0-9]*]] = {{.*}}[[MODULES:![0-9]*]], metadata !""} ; [ DW_TAG_compile_unit ]
// CHECK: [[FILE:![0-9]*]] {{.*}}debug-info-namespace.cpp"
// CHECK: [[FOOCPP:![0-9]*]] = metadata !{metadata !"foo.cpp", {{.*}}
@@ -48,6 +58,8 @@
// CHECK: [[FUNC:![0-9]*]] {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] [func]
// CHECK: [[FILE2]]} ; [ DW_TAG_file_type ] [{{.*}}foo.cpp]
// CHECK: [[I:![0-9]*]] = {{.*}}, metadata [[NS]], metadata !"i", {{.*}} ; [ DW_TAG_variable ] [i]
+// CHECK: [[VAR:![0-9]*]] = {{.*}}, metadata [[SPACE:![0-9]*]], metadata !"j", {{.*}} ; [ DW_TAG_variable ] [j]
+// CHECK: [[SPACE]] = {{.*}}, metadata !"C", {{.*}} ; [ DW_TAG_namespace ] [C]
// CHECK: [[MODULES]] = metadata !{metadata [[M1:![0-9]*]], metadata [[M2:![0-9]*]], metadata [[M3:![0-9]*]], metadata [[M4:![0-9]*]], metadata [[M5:![0-9]*]], metadata [[M6:![0-9]*]], metadata [[M7:![0-9]*]], metadata [[M8:![0-9]*]], metadata [[M9:![0-9]*]], metadata [[M10:![0-9]*]], metadata [[M11:![0-9]*]], metadata [[M12:![0-9]*]]}
// CHECK: [[M1]] = metadata !{i32 {{[0-9]*}}, metadata [[CTXT]], metadata [[NS]], i32 11} ; [ DW_TAG_imported_module ]
// CHECK: [[M2]] = metadata !{i32 {{[0-9]*}}, metadata [[CU]], metadata [[CTXT]], i32 {{[0-9]*}}} ; [ DW_TAG_imported_module ]
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
