[PATCH] D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass.

[Alexey Lapshin via Phabricator via llvm-commits]

avl marked an inline comment as done.
avl added a comment.

OK, I will modify testcase to be single pass.



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3163-3164
     if (DII->getParent() == SrcBlock) {
-      // dbg.value is in the same basic block as the sunk inst, see if we can
-      // salvage it. Clone a new copy of the instruction: on success we need
-      // both salvaged and unsalvaged copies.
-      SmallVector<DbgVariableIntrinsic *, 1> TmpUser{
-          cast<DbgVariableIntrinsic>(DII->clone())};
-
-      if (!salvageDebugInfoForDbgValues(*I, TmpUser)) {
-        // We are unable to salvage: sink the cloned dbg.value, and mark the
-        // original as undef, terminating any earlier variable location.
-        LLVM_DEBUG(dbgs() << "SINK: " << *DII << '\n');
-        TmpUser[0]->insertBefore(&*InsertPos);
-        Value *Undef = UndefValue::get(I->getType());
-        DII->setOperand(0, MetadataAsValue::get(DII->getContext(),
-                                                ValueAsMetadata::get(Undef)));
+      if (DII->getIntrinsicID() == Intrinsic::dbg_declare &&
+          !isa<AllocaInst>(I)) {
+        // There should be only one dbg.declare instruction. Thus we could not
----------------
rnk wrote:
> aprantl wrote:
> > jmorse wrote:
> > > Interesting -- are there circumstances where this happens (dbg.declare of something that isn't an alloca)? I suspect it would be IR that didn't make sense, we might be better off disallowing it in the IR Verifier instead.
> > I must admit I'm confused how this patch relates to the example in the review description. What is the instruction coming in here in the example?
> clang emits dbg.declare of things that are not allocas from time to time. Usually it is for C++ classes that, when passed by value at the source level, are instead passed indirectly by pointer as required by the ABI. Or, for POD structs passed using byval.
> 
> Also, after inlining, these arguments may resolve to pointers to non-alloca locations, as in `this->field = getSRetThing();`.
> 
That happens with this patch and llvm/test/DebugInfo/X86/sroa-after-inlining2.ll testcase if InstructionCombining.cpp:3163-3173 piece deleted.

opt sroa-after-inlining2.ll -sroa -instcombine -inline -instcombine -sroa -S -o -

before second instcombine pass there is following IR : 


```
entry:
  %result = alloca i64, align 8
  %tmpcast = bitcast i64* %result to %struct.S1*     <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  %0 = bitcast i64* %result to i8*, !dbg !24
  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0), !dbg !24
  call void @llvm.dbg.declare(metadata %struct.S1* %tmpcast, metadata !12, metadata !DIExpression()), !dbg !25   <<<<<<<<<<
```

after second instcombine pass it transformed into : 


```
entry:
  %result = alloca i64, align 8
  %0 = bitcast i64* %result to i8*, !dbg !24
  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0), !dbg !24
  call void @llvm.dbg.declare(metadata i64* %result, metadata !12, metadata !DIExpression()), !dbg !25


if.end:                                           ; preds = %entry
  %tmpcast = bitcast i64* %result to %struct.S1*   <<<<<<<<<<<<<<<<<<<<<<<<<
  call void @llvm.dbg.declare(metadata %struct.S1* %tmpcast, metadata !12, metadata !DIExpression()), !dbg !25   <<<<<<<<<<
```

instcombine moves bitcast instruction and clones dbg.declare.


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

https://reviews.llvm.org/D64595