Debug Info: Handle by-reference arguments correctly during inlining.

[aprantl]

Summary

If a function argument that is passed by reference is remapped to an alloca in the calling function, eliminate the (now) superfluous DW_OP_deref from the DIExpression.

Depends on http://reviews.llvm.org/D6986.

[dblaikie]

lib/Transforms/Utils/CloneFunction.cpp

291

Does createExpression need a DIBuilder? Does it just need access to a Module? (perhaps it could just be a standalone non-member function that takes a Module?)

293

So if during inlining we take a pointer and remove the indirection (eg: a function has "int *x" and "*x + 3" and its called with "int y = ...; func(&y)", etc) we drop the expression entirely? Because we can't describe the value of the pointer when there's nothing to point to, I take it? (that seems correct, just checking I understand correctly)

Is that case tested?

296

These two functions have a lot in common (everything except "getAddress/getValue") - should they be combined in some way (should DbgValueInst/DbgDeclareInst have a common "get" (address or value) for ease of access/consistency - or does that not make sense?)? If not, then at least the rest of the function could be pulled out into a common function or template if necessary)

test/DebugInfo/inline-indirect-arg.ll

4

This seems like a lot of code to test this scenario. Could this be simplified further? (I'd also expect to see some attribute((always_inline)) so you don't have to rely on the inliner to pick the right things to inline (You can compile the example with -mllvm -disable-llvm-optzns to get the bitcode before the AlwaysInliner runs (since it'll run at -O0 and a simple "clang -emit-llvm" will get you post-inlined IR, which isn't what you want)))

[aprantl]

I just verified that this is no longer applicable as there is no longer a DW_OP_deref in the inlined function.

lib/Transforms/Utils/CloneFunction.cpp

 #include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
public:
 
     /// CloneBlock - The specified block is found to be reachable, clone it and
     /// anything that it can reach.
     void CloneBlock(const BasicBlock *BB,
                     std::vector<const BasicBlock*> &ToClone);
   };
 }
 
+
+/// Return a new DIExpression without the leading DW_OP_deref.
+static DIExpression stripDeref(DIExpression Expr, Module &M) {
+  if (Expr.getNumElements() >= 1 &&
+      Expr.getElement(0) == dwarf::DW_OP_deref) {
+    SmallVector<int64_t, 8> Ops;
+    for (unsigned i = 1; i < Expr.getNumElements(); ++i)
+      Ops.push_back(Expr.getElement(i));
+    return DIBuilder(M, false).createExpression(Ops);
+  }
+  return DIExpression();
+}
+
+static void updateDebugExpression(const DbgDeclareInst *Orig, DbgDeclareInst *New,
+                                  Module &M) {
+  if (isa<Argument>(Orig->getAddress()) && isa<AllocaInst>(New->getAddress()))
+    if (auto E = stripDeref(DIExpression(New->getExpression()), M))
+      New->setOperand(2, MetadataAsValue::get(New->getContext(), E));
+}
+
+static void updateDebugExpression(const DbgValueInst *Orig, DbgValueInst *New,
+                                  Module &M) {
+  if (isa<Argument>(Orig->getValue()) && isa<AllocaInst>(New->getValue()))
+    if (auto E = stripDeref(DIExpression(New->getExpression()), M))
+      New->setOperand(3, MetadataAsValue::get(New->getContext(), E));
+}
+
 /// CloneBlock - The specified block is found to be reachable, clone it and
 /// anything that it can reach.
 void PruningFunctionCloner::CloneBlock(const BasicBlock *BB,
                                        std::vector<const BasicBlock*> &ToClone){
   WeakVH &BBEntry = VMap[BB];
 
   // Have we already cloned this block?
   if (BBEntry) return;
for (BasicBlock::const_iterator II = BB->begin(), IE = --BB->end();
     Instruction *NewInst = II->clone();
 
     // Eagerly remap operands to the newly cloned instruction, except for PHI
     // nodes for which we defer processing until we update the CFG.
     if (!isa<PHINode>(NewInst)) {
       RemapInstruction(NewInst, VMap,
                        ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges);
 
+      // If an indirect argument has been remapped to an alloca, unwrap the
+      // debug info expression.
+      Module &M = *NewFunc->getParent();
+      if (auto DDI = dyn_cast<DbgDeclareInst>(NewInst))
+        updateDebugExpression(cast<DbgDeclareInst>(II), DDI, M);
+      else if (auto DVI = dyn_cast<DbgValueInst>(NewInst))
+        updateDebugExpression(cast<DbgValueInst>(II), DVI, M);
+
       // If we can simplify this instruction to some other value, simply add
       // a mapping to that value rather than inserting a new instruction into
       // the basic block.
       if (Value *V = SimplifyInstruction(NewInst, DL)) {
         // On the off-chance that this simplifies to an instruction in the old
         // function, map it back into the new function.
         if (Value *MappedV = VMap.lookup(V))
           V = MappedV;

test/DebugInfo/inline-indirect-arg.ll

+; RUN: opt -inline %s -S | FileCheck %s
+; ModuleID = 'test.cpp'
+; clang++ -g -std=c++11  -O0  -fno-exceptions -fno-rtti -S -emit-llvm test.cpp  -o test.ll
+; struct B {
+;   long a, b, c;
+;   void operator!=(B);
+; };
+; long fn3();
+; static B fn1() {
+;   B i;
+;   i.c = fn3();
+;   return i;
+; }
+; void fn2() {
+;   B b, e = fn1();
+;   b != e;
+; }
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.10.0"
+
+%struct.B = type { i64, i64, i64 }
+
+; Function Attrs: nounwind ssp uwtable
+define void @_Z3fn2v() #0 {
+entry:
+  %b = alloca %struct.B, align 8
+  %e = alloca %struct.B, align 8
+  %agg.tmp = alloca %struct.B, align 8
+  call void @llvm.dbg.declare(metadata %struct.B* %b, metadata !26, metadata !27), !dbg !28
+  call void @llvm.dbg.declare(metadata %struct.B* %e, metadata !29, metadata !27), !dbg !30
+  ; Neither of these dbg.declares should use a DW_OP_deref.
+  ; CHECK: call void @llvm.dbg.declare(metadata %struct.B* %e, metadata ![[I:.*]], metadata ![[EXPR:.*]])
+  ; CHECK: call void @llvm.dbg.declare(metadata %struct.B* %e, metadata ![[I]], metadata ![[EXPR]])
+  ; CHECK: ![[EXPR]] = !{!"0x102"} ; [ DW_TAG_expression ]
+  ; CHECK: ![[E]] = {{.*}}; [ DW_TAG_auto_variable ] [e] [line 12]
+  ; CHECK: ![[I]] = {{.*}}; [ DW_TAxG_auto_variable ] [i] [line 7]
+  call void @_ZL3fn1v(%struct.B* sret %e), !dbg !31
+  %0 = bitcast %struct.B* %agg.tmp to i8*, !dbg !32
+  %1 = bitcast %struct.B* %e to i8*, !dbg !32
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %0, i8* %1, i64 24, i32 8, i1 false), !dbg !32
+  call void @_ZN1BneES_(%struct.B* %b, %struct.B* byval align 8 %agg.tmp), !dbg !32
+  ret void, !dbg !33
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+; Function Attrs: nounwind ssp uwtable
+define internal void @_ZL3fn1v(%struct.B* noalias sret %agg.result) #0 {
+entry:
+  call void @llvm.dbg.declare(metadata %struct.B* %agg.result, metadata !34, metadata !35), !dbg !36
+  %call = call i64 @_Z3fn3v(), !dbg !37
+  %c = getelementptr inbounds %struct.B* %agg.result, i32 0, i32 2, !dbg !38
+  store i64 %call, i64* %c, align 8, !dbg !38
+  ret void, !dbg !39
+}
+
+declare void @_ZN1BneES_(%struct.B*, %struct.B* byval align 8)
+
+; Function Attrs: nounwind
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1) #3
+
+declare i64 @_Z3fn3v()
+
+attributes #0 = { nounwind ssp uwtable }
+attributes #1 = { nounwind readnone }
+attributes #3 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!22, !23, !24}
+!llvm.ident = !{!25}
+
+!0 = !{!"0x11\004\00clang version 3.6.0 (trunk 225982) (llvm/trunk 226003)\000\00\000\00\001", !1, !2, !3, !14, !2, !2} ; [ DW_TAG_compile_unit ] [/test.cpp] [DW_LANG_C_plus_plus]
+!1 = !{!"test.cpp", !""}
+!2 = !{}
+!3 = !{!4}
+!4 = !{!"0x13\00B\001\00192\0064\000\000\000", !1, null, null, !5, null, null, !"_ZTS1B"} ; [ DW_TAG_structure_type ] [B] [line 1, size 192, align 64, offset 0] [def] [from ]
+!5 = !{!6, !8, !9, !10}
+!6 = !{!"0xd\00a\002\0064\0064\000\000", !1, !"_ZTS1B", !7} ; [ DW_TAG_member ] [a] [line 2, size 64, align 64, offset 0] [from long int]
+!7 = !{!"0x24\00long int\000\0064\0064\000\000\005", null, null} ; [ DW_TAG_base_type ] [long int] [line 0, size 64, align 64, offset 0, enc DW_ATE_signed]
+!8 = !{!"0xd\00b\002\0064\0064\0064\000", !1, !"_ZTS1B", !7} ; [ DW_TAG_member ] [b] [line 2, size 64, align 64, offset 64] [from long int]
+!9 = !{!"0xd\00c\002\0064\0064\00128\000", !1, !"_ZTS1B", !7} ; [ DW_TAG_member ] [c] [line 2, size 64, align 64, offset 128] [from long int]
+!10 = !{!"0x2e\00operator!=\00operator!=\00_ZN1BneES_\003\000\000\000\000\00256\000\003", !1, !"_ZTS1B", !11, null, null, null, null, null} ; [ DW_TAG_subprogram ] [line 3] [operator!=]
+!11 = !{!"0x15\00\000\000\000\000\000\000", null, null, null, !12, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!12 = !{null, !13, !"_ZTS1B"}
+!13 = !{!"0xf\00\000\0064\0064\000\001088\00", null, null, !"_ZTS1B"} ; [ DW_TAG_pointer_type ] [line 0, size 64, align 64, offset 0] [artificial] [from _ZTS1B]
+!14 = !{!15, !19}
+!15 = !{!"0x2e\00fn2\00fn2\00_Z3fn2v\0011\000\001\000\000\00256\000\0011", !1, !16, !17, null, void ()* @_Z3fn2v, null, null, !2} ; [ DW_TAG_subprogram ] [line 11] [def] [fn2]
+!16 = !{!"0x29", !1}                              ; [ DW_TAG_file_type ] [/test.cpp]
+!17 = !{!"0x15\00\000\000\000\000\000\000", null, null, null, !18, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!18 = !{null}
+!19 = !{!"0x2e\00fn1\00fn1\00_ZL3fn1v\006\001\001\000\000\00256\000\006", !1, !16, !20, null, void (%struct.B*)* @_ZL3fn1v, null, null, !2} ; [ DW_TAG_subprogram ] [line 6] [local] [def] [fn1]
+!20 = !{!"0x15\00\000\000\000\000\000\000", null, null, null, !21, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!21 = !{!"_ZTS1B"}
+!22 = !{i32 2, !"Dwarf Version", i32 2}
+!23 = !{i32 2, !"Debug Info Version", i32 2}
+!24 = !{i32 1, !"PIC Level", i32 2}
+!25 = !{!"clang version 3.6.0 (trunk 225982) (llvm/trunk 226003)"}
+!26 = !{!"0x100\00b\0012\000", !15, !16, !"_ZTS1B"} ; [ DW_TAG_auto_variable ] [b] [line 12]
+!27 = !{!"0x102"}                                 ; [ DW_TAG_expression ]
+!28 = !{i32 12, i32 5, !15, null}
+!29 = !{!"0x100\00e\0012\000", !15, !16, !"_ZTS1B"} ; [ DW_TAG_auto_variable ] [e] [line 12]
+!30 = !{i32 12, i32 8, !15, null}
+!31 = !{i32 12, i32 12, !15, null}
+!32 = !{i32 13, i32 3, !15, null}
+!33 = !{i32 14, i32 1, !15, null}
+!34 = !{!"0x100\00i\007\000", !19, !16, !"_ZTS1B"} ; [ DW_TAG_auto_variable ] [i] [line 7]
+!35 = !{!"0x102\006"}                             ; [ DW_TAG_expression ] [DW_OP_deref]
+!36 = !{i32 7, i32 5, !19, null}
+!37 = !{i32 8, i32 9, !19, null}
+!38 = !{i32 8, i32 3, !19, null}
+!39 = !{i32 9, i32 3, !19, null}