PR32382: Adapt to LLVM changes in DIExpression.

[aprantl]

Summary

LLVM has changes the semantics of dbg.declare for describing function arguments. After this patch a dbg.declare always takes the *address* of a variable as the first argument, even if the argument is not an alloca.

https://bugs.llvm.org/show_bug.cgi?id=32382
rdar://problem/31205000

[dblaikie]

I'm a bit confused - the alloca was only emitted at -O0, by the looks of it. Presumably it's pessimizing in some way at higher optimization levels? Or is that not the case?

Also, it looks like this change lost the "if > gmlt" test, so might cause variable declarations (& thus types) to leak into GMLT, which is undesirable.

[aprantl]

In D31440#713308, @dblaikie wrote:

I'm a bit confused - the alloca was only emitted at -O0, by the looks of it. Presumably it's pessimizing in some way at higher optimization levels? Or is that not the case?

I think it is really working around the odd behavior of LLVM here. What gives it away is the we key the addition of the extra DW_OP_deref on whether it is an alloca or not. But note that this is not how dbg.declare works: dbg.declare(%alloca, !DIExpression()) is (kind of) equivalent to dbg.value(%alloca, !DIExpression(DW_OP_deref)). So we are using the presence of the alloca as a proxy for how the backend happens to compile the DwarfExpression here and work around its idiosyncrasy by emitting an extra DW_OP_deref.

Also, it looks like this change lost the "if > gmlt" test, so might cause variable declarations (& thus types) to leak into GMLT, which is undesirable.

Oh.. that was unintentional collateral damage. Thanks for noticing!

[aprantl]

Add accidentally removed check for the debug info level back in.

[dblaikie]

In D31440#713396, @aprantl wrote:

In D31440#713308, @dblaikie wrote:

I'm a bit confused - the alloca was only emitted at -O0, by the looks of it. Presumably it's pessimizing in some way at higher optimization levels? Or is that not the case?

I think it is really working around the odd behavior of LLVM here. What gives it away is the we key the addition of the extra DW_OP_deref on whether it is an alloca or not. But note that this is not how dbg.declare works: dbg.declare(%alloca, !DIExpression()) is (kind of) equivalent to dbg.value(%alloca, !DIExpression(DW_OP_deref)). So we are using the presence of the alloca as a proxy for how the backend happens to compile the DwarfExpression here and work around its idiosyncrasy by emitting an extra DW_OP_deref.

I'm still not really following, sorry - perhaps it'd help me if you could describe the state of things in ToT currently, the state this change attempts to create, and the state (if distinct from the previous) that might be ideal. (it's not clear to me if there's still a "workaround" after your change - sounds like there should be/is, if previously there was no alloca above O0 but after this change there will be... )

Also, it looks like this change lost the "if > gmlt" test, so might cause variable declarations (& thus types) to leak into GMLT, which is undesirable.

Oh.. that was unintentional collateral damage. Thanks for noticing!

[echristo]

Sounds like Dave is asking for changes so I'll put it down as Request Changes to get it out of my queue. :)

[aprantl]

I think I may haved screwed up the phabricator process for this one.
This landed as r300523.

lib/CodeGen/CGDebugInfo.cpp

void CGDebugInfo::EmitDeclare(const VarDecl *VD, llvm::Value *Storage,
   AppendAddressSpaceXDeref(AddressSpace, Expr);
 
   // If this is the first argument and it is implicit then
   // give it an object pointer flag.
   // FIXME: There has to be a better way to do this, but for static
   // functions there won't be an implicit param at arg1 and
   // otherwise it is 'self' or 'this'.
   if (isa<ImplicitParamDecl>(VD) && ArgNo && *ArgNo == 1)
-    Flags |= llvm::DINode::FlagObjectPointer;
-  if (auto *Arg = dyn_cast<llvm::Argument>(Storage))
-    if (Arg->getType()->isPointerTy() && !Arg->hasByValAttr() &&
-        !VD->getType()->isPointerType())
-      Expr.push_back(llvm::dwarf::DW_OP_deref);
 
+  // Note: Older versions of clang used to emit byval references with an extra
+  // DW_OP_deref, because they referenced the IR arg directly instead of
+  // referencing an alloca. Newer versions of LLVM don't treat allocas
+  // differently from other function arguments when used in a dbg.declare.
   auto *Scope = cast<llvm::DIScope>(LexicalBlockStack.back());
-
   StringRef Name = VD->getName();
   if (!Name.empty()) {
     if (VD->hasAttr<BlocksAttr>()) {
+      // Here, we need an offset *into* the alloca.
       CharUnits offset = CharUnits::fromQuantity(32);
       Expr.push_back(llvm::dwarf::DW_OP_plus);
       // offset of __forwarding field
       offset = CGM.getContext().toCharUnitsFromBits(
           CGM.getTarget().getPointerWidth(0));
       Expr.push_back(offset.getQuantity());
       Expr.push_back(llvm::dwarf::DW_OP_deref);
       Expr.push_back(llvm::dwarf::DW_OP_plus);
       // offset of x field
       offset = CGM.getContext().toCharUnitsFromBits(XOffset);
       Expr.push_back(offset.getQuantity());
-
-      // Create the descriptor for the variable.
-      auto *D = ArgNo
-                    ? DBuilder.createParameterVariable(Scope, VD->getName(),
-                                                       *ArgNo, Unit, Line, Ty)
-                    : DBuilder.createAutoVariable(Scope, VD->getName(), Unit,
-                                                  Line, Ty, Align);
-
-      // Insert an llvm.dbg.declare into the current block.
-      DBuilder.insertDeclare(
-          Storage, D, DBuilder.createExpression(Expr),
-          llvm::DebugLoc::get(Line, Column, Scope, CurInlinedAt),
-          Builder.GetInsertBlock());
-      return;
-    } else if (isa<VariableArrayType>(VD->getType()))
-      Expr.push_back(llvm::dwarf::DW_OP_deref);
+    }
   } else if (const auto *RT = dyn_cast<RecordType>(VD->getType())) {
     // If VD is an anonymous union then Storage represents value for
     // all union fields.
     const auto *RD = cast<RecordDecl>(RT->getDecl());
     if (RD->isUnion() && RD->isAnonymousStructOrUnion()) {
       // GDB has trouble finding local variables in anonymous unions, so we emit
       // artifical local variables for each of the members.
       //
void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
 
   const llvm::DataLayout &target = CGM.getDataLayout();
 
   CharUnits offset = CharUnits::fromQuantity(
       target.getStructLayout(blockInfo.StructureType)
           ->getElementOffset(blockInfo.getCapture(VD).getIndex()));
 
   SmallVector<int64_t, 9> addr;
-  if (isa<llvm::AllocaInst>(Storage))
-    addr.push_back(llvm::dwarf::DW_OP_deref);
   addr.push_back(llvm::dwarf::DW_OP_plus);
   addr.push_back(offset.getQuantity());
   if (isByRef) {
     addr.push_back(llvm::dwarf::DW_OP_deref);
     addr.push_back(llvm::dwarf::DW_OP_plus);
     // offset of __forwarding field
     offset =
         CGM.getContext().toCharUnitsFromBits(target.getPointerSizeInBits(0));
void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   auto *D = DBuilder.createAutoVariable(
       cast<llvm::DILocalScope>(LexicalBlockStack.back()), VD->getName(), Unit,
       Line, Ty, false, llvm::DINode::FlagZero, Align);
 
   // Insert an llvm.dbg.declare into the current block.
   auto DL =
       llvm::DebugLoc::get(Line, Column, LexicalBlockStack.back(), CurInlinedAt);
+  auto *Expr = DBuilder.createExpression(addr);
   if (InsertPoint)
-    DBuilder.insertDeclare(Storage, D, DBuilder.createExpression(addr), DL,
-                           InsertPoint);
+    DBuilder.insertDeclare(Storage, D, Expr, DL, InsertPoint);
   else
-    DBuilder.insertDeclare(Storage, D, DBuilder.createExpression(addr), DL,
-                           Builder.GetInsertBlock());
+    DBuilder.insertDeclare(Storage, D, Expr, DL, Builder.GetInsertBlock());
 }
 
 void CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI,
                                            unsigned ArgNo,
                                            CGBuilderTy &Builder) {
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   EmitDeclare(VD, AI, ArgNo, Builder);
 }

test/CodeGen/debug-info-vla.c

 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple x86_64-apple-darwin %s -o - | FileCheck %s
 
 void testVLAwithSize(int s)
 {
 // CHECK: dbg.declare
 // CHECK: dbg.declare({{.*}}, metadata ![[VAR:.*]], metadata ![[EXPR:.*]])
-// CHECK: ![[VAR]] = !DILocalVariable(name: "vla",{{.*}} line: [[@LINE+2]]
-// CHECK: ![[EXPR]] = !DIExpression(DW_OP_deref)
+// CHECK: ![[EXPR]] = !DIExpression()
+// CHECK: ![[VAR]] = !DILocalVariable(name: "vla",{{.*}} line: [[@LINE+1]]
   int vla[s];
   int i;
   for (i = 0; i < s; i++) {
     vla[i] = i*i;
   }
 }

test/CodeGenCXX/debug-info.cpp

 // CHECK-NOT:                             line:
 // CHECK-NOT:                             size:
 // CHECK-NOT:                             align:
 // CHECK-NOT:                             offset:
 // CHECK-SAME:                            baseType: ![[INCTYPE:[0-9]+]]
 
 // CHECK: ![[INCTYPE]] = !DICompositeType(tag: DW_TAG_structure_type, name: "incomplete"
 // CHECK-SAME:                                   DIFlagFwdDecl
+// CHECK: ![[EXPR]] = !DIExpression()
 
 template<typename T> struct Identity {
   typedef T Type;
 };
 
 void f(Identity<int>::Type a) {}
 void f(Identity<int> a) {}
 void f(int& a) { }

 // CHECK-SAME:             scope: [[PR14763]]
 // CHECK-SAME:             identifier:
   foo(const foo&);
 };
 
 // For some reason function arguments ended up down here
 // CHECK: ![[F]] = !DILocalVariable(name: "f", arg: 1, scope: ![[FUNC]]
 // CHECK-SAME:                      type: ![[FOO]]
-// CHECK: ![[EXPR]] = !DIExpression(DW_OP_deref)
 foo func(foo f) {
   return f; // reference 'f' for now because otherwise we hit another bug
 }
 
 }
 
 void foo() {
 // CHECK: !DILocalVariable(name: "c"

test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl

 // RUN: %clang -cl-std=CL2.0 -emit-llvm -g -O0 -S -target amdgcn-amd-amdhsa -mcpu=fiji -o - %s | FileCheck %s
 
-// CHECK-DAG: ![[NONE:[0-9]+]] = !DIExpression()
 // CHECK-DAG: ![[LOCAL:[0-9]+]] = !DIExpression(DW_OP_constu, 2, DW_OP_swap, DW_OP_xderef)
 // CHECK-DAG: ![[PRIVATE:[0-9]+]] = !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef)
 
 // CHECK-DAG: ![[FILEVAR0:[0-9]+]] = distinct !DIGlobalVariable(name: "FileVar0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}, isLocal: false, isDefinition: true)
 // CHECK-DAG: !DIGlobalVariableExpression(var: ![[FILEVAR0]])
 global int *FileVar0;
 // CHECK-DAG: ![[FILEVAR1:[0-9]+]] = distinct !DIGlobalVariable(name: "FileVar1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}, isLocal: false, isDefinition: true)
 // CHECK-DAG: !DIGlobalVariableExpression(var: ![[FILEVAR1]])
kernel void kernel1(
   // CHECK-DAG: ![[FUNCVAR3:[0-9]+]] = !DILocalVariable(name: "FuncVar3", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
   // CHECK-DAG: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[FUNCVAR3]], metadata ![[PRIVATE]]), !dbg !{{[0-9]+}}
   private int *FuncVar3 = Tmp0;
   // CHECK-DAG: ![[FUNCVAR4:[0-9]+]] = !DILocalVariable(name: "FuncVar4", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
   // CHECK-DAG: call void @llvm.dbg.declare(metadata i32 addrspace(4)** {{.*}}, metadata ![[FUNCVAR4]], metadata ![[PRIVATE]]), !dbg !{{[0-9]+}}
   int *FuncVar4 = Tmp1;
 
   // CHECK-DAG: ![[FUNCVAR5:[0-9]+]] = !DILocalVariable(name: "FuncVar5", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
-  // CHECK-DAG: call void @llvm.dbg.declare(metadata i32 addrspace(1)** {{.*}}, metadata ![[FUNCVAR5]], metadata ![[NONE]]), !dbg !{{[0-9]+}}
+  // CHECK-DAG: call void @llvm.dbg.declare(metadata i32 addrspace(1)** {{.*}}, metadata ![[FUNCVAR5]], metadata ![[NONE:[0-9]+]]), !dbg !{{[0-9]+}}
   global int *constant FuncVar5 = KernelArg0;
   // CHECK-DAG: ![[FUNCVAR6:[0-9]+]] = !DILocalVariable(name: "FuncVar6", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
   // CHECK-DAG: call void @llvm.dbg.declare(metadata i32 addrspace(2)** {{.*}}, metadata ![[FUNCVAR6]], metadata ![[NONE]]), !dbg !{{[0-9]+}}
   constant int *constant FuncVar6 = KernelArg1;
   // CHECK-DAG: ![[FUNCVAR7:[0-9]+]] = !DILocalVariable(name: "FuncVar7", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
   // CHECK-DAG: call void @llvm.dbg.declare(metadata i32 addrspace(3)** {{.*}}, metadata ![[FUNCVAR7]], metadata ![[NONE]]), !dbg !{{[0-9]+}}
   local int *constant FuncVar7 = KernelArg2;
   // CHECK-DAG: ![[FUNCVAR8:[0-9]+]] = !DILocalVariable(name: "FuncVar8", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})