Hi David,
  Thanks for reviewing my patch. I am uploading a new patch that addresses your 
questions.

  > Can "VarOrInit" ever be null? If so, can you explain when/why? If not, use 
"dyn_cast" instead of dyn_cast_or_null.


  VarOrInit cannot be null. I have switched to using dyn_cast as you suggested.


  > if possible, even push the LinkageName stuff into the same conditional:
  if (GlobalVariable *Var = dyn_cast...) {
  setLocation
  isLocalToUnit = ...
  if (D->getDeclContext() && !isa<functionDecl> ... )
    LinkageName = Var->getName();
  }


  Fixed.

  - Gao.

http://llvm-reviews.chandlerc.com/D1281

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1281?vs=3328&id=3583#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,30 @@
 }
 
 /// 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.
+  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 +3076,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 +3112,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,17 @@
 // 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 in function foo()
+// CHECK: [ DW_TAG_variable ] [d] [line 6] [def]
+const int d = 100;
+extern int foo();
+
 // Two variables with same name in separate scope.
 // Radar 8330217.
 int main() {
+// CHECK-NOT: [ DW_TAG_variable ] [d] [line 14]
+// CHECK: [ DW_TAG_auto_variable ] [d] [line 14]
+	const int d = 4;
 	int j = 0;
 	int k = 0;
 // CHECK: DW_TAG_auto_variable ] [i]
@@ -12,5 +22,10 @@
 // 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
 }
+
+int foo()
+{
+  return d;
+}
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,22 @@
+// RUN: %clangxx -g -gcolumn-info -O0 %s -c -o %t.o
+// RUN: llvm-dwarfdump %t.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: Address            Line   Column File   ISA Flags
+// CHECK:                    14     14      1   0  is_stmt prologue_end
+// CHECK:                    15      3      1   0  is_stmt
+// CHECK:                    15      3      1   0  is_stmt end_sequence
Index: test/CodeGenCXX/debug-info-namespace.cpp
===================================================================
--- test/CodeGenCXX/debug-info-namespace.cpp
+++ test/CodeGenCXX/debug-info-namespace.cpp
@@ -36,6 +36,16 @@
   return i + X::B::i + Y::B::i;
 }
 
+namespace monkey
+{
+  const int ape = 32;
+}
+
+int test()
+{
+  return monkey::ape;
+}
+
 // This should work even if 'i' and 'func' were declarations & not definitions,
 // but it doesn't yet.
 
@@ -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 !"ape", {{.*}} ; [ DW_TAG_variable ] [ape]
+// CHECK: [[SPACE]] = {{.*}}, metadata !"monkey", {{.*}} ; [ DW_TAG_namespace ] [monkey]
 // 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

Reply via email to