[LSR] Expand: Use the replaced value's debug loc (PR25630)

[vsk]

Summary

Fix PR25630 by setting the debug location of code generated in
LSRInstance::Expand() to the location attached to the replaced value.

Depends on D39985

[qcolombet]

LGTM. Nitpicks below.

test/Transforms/LoopStrengthReduce/pr25630.ll

1

Could you use a more descriptive name for the filename?
Then, mention the PR in the comment here.

[aprantl]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

4940

What besides an instruction could this be? An Argument?

test/Transforms/LoopStrengthReduce/pr25630.ll

57

it might make sense to remove all the dbg.value intrinsics that are not used by the test and remove all the metadata that depends on them to make the test easier to read/maintain

89

we usually strip out all non-essential attributes from tests

[vsk]

• Address review feedback from Quentin and Adrian.
• Add test coverage for the logic in GenerateIVChain, and for more users of LSRInstance::Expand.

[vsk]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

4940

I'm not sure, but I haven't been able to prove that this is always an Instruction.

test/Transforms/LoopStrengthReduce/pr25630.ll

1

Will fix.

57

Will fix.

89

Will fix.

[aprantl]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

4940

Might as well use a cast<> then, it will assert and we'll find out :-)
Hm.. it could be a constant, perhaps?

[vsk]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

4940

If we use a cast<>, test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll fails because the operand val is an i8*.

[aprantl]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

4940

thanks for checking!

[davide]

This one LGTM, thanks Vedant!

lib/Transforms/Scalar/LoopStrengthReduce.cpp

void LSRInstance::GenerateIVChain(const IVChain &Chain, SCEVExpander &Rewriter,
   for (const IVInc &Inc : Chain) {
     Instruction *InsertPt = Inc.UserInst;
     if (isa<PHINode>(InsertPt))
       InsertPt = L->getLoopLatch()->getTerminator();
 
     // IVOper will replace the current IV User's operand. IVSrc is the IV
     // value currently held in a register.
     Value *IVOper = IVSrc;
+    DebugLoc Loc = InsertPt->getDebugLoc();
     if (!Inc.IncExpr->isZero()) {
       // IncExpr was the result of subtraction of two narrow values, so must
       // be signed.
       const SCEV *IncExpr = SE.getNoopOrSignExtend(Inc.IncExpr, IntTy);
       LeftOverExpr = LeftOverExpr ?
         SE.getAddExpr(LeftOverExpr, IncExpr) : IncExpr;
     }
     if (LeftOverExpr && !LeftOverExpr->isZero()) {
       // Expand the IV increment.
       Rewriter.clearPostInc();
-      Value *IncV = Rewriter.expandCodeFor(LeftOverExpr, IntTy, InsertPt);
+      Value *IncV = Rewriter.expandCodeFor(LeftOverExpr, Loc, IntTy, InsertPt);
       const SCEV *IVOperExpr = SE.getAddExpr(SE.getUnknown(IVSrc),
                                              SE.getUnknown(IncV));
-      IVOper = Rewriter.expandCodeFor(IVOperExpr, IVTy, InsertPt);
+      IVOper = Rewriter.expandCodeFor(IVOperExpr, Loc, IVTy, InsertPt);
 
       // If an IV increment can't be folded, use it as the next IV value.
       if (!canFoldIVIncExpr(LeftOverExpr, Inc.UserInst, Inc.IVOperand, TTI)) {
         assert(IVTy == IVOper->getType() && "inconsistent IV increment type");
         IVSrc = IVOper;
         LeftOverExpr = nullptr;
       }
     }
else if (SE.getEffectiveSCEVType(Ty) == SE.getEffectiveSCEVType(OpTy))
     // Expand directly to the ultimate type if it's the right size.
     Ty = OpTy;
   // This is the type to do integer arithmetic in.
   Type *IntTy = SE.getEffectiveSCEVType(Ty);
 
   // Build up a list of operands to add together to form the full base.
   SmallVector<const SCEV *, 8> Ops;
 
+  // Find a debug location for the expanded instructions.
+  DebugLoc Loc;
+  if (auto *I = dyn_cast<Instruction>(LF.OperandValToReplace))
+    Loc = I->getDebugLoc();
+
   // Expand the BaseRegs portion.
   for (const SCEV *Reg : F.BaseRegs) {
     assert(!Reg->isZero() && "Zero allocated in a base register!");
 
     // If we're expanding for a post-inc user, make the post-inc adjustment.
     Reg = denormalizeForPostIncUse(Reg, LF.PostIncLoops, SE);
-    Ops.push_back(SE.getUnknown(Rewriter.expandCodeFor(Reg, nullptr)));
+    Ops.push_back(SE.getUnknown(Rewriter.expandCodeFor(Reg, Loc, nullptr)));
   }
 
   // Expand the ScaledReg portion.
   Value *ICmpScaledV = nullptr;
   if (F.Scale != 0) {
     const SCEV *ScaledS = F.ScaledReg;
 
     // If we're expanding for a post-inc user, make the post-inc adjustment.
     PostIncLoopSet &Loops = const_cast<PostIncLoopSet &>(LF.PostIncLoops);
     ScaledS = denormalizeForPostIncUse(ScaledS, Loops, SE);
 
     if (LU.Kind == LSRUse::ICmpZero) {
       // Expand ScaleReg as if it was part of the base regs.
       if (F.Scale == 1)
         Ops.push_back(
-            SE.getUnknown(Rewriter.expandCodeFor(ScaledS, nullptr)));
+            SE.getUnknown(Rewriter.expandCodeFor(ScaledS, Loc, nullptr)));
       else {
         // An interesting way of "folding" with an icmp is to use a negated
         // scale, which we'll implement by inserting it into the other operand
         // of the icmp.
         assert(F.Scale == -1 &&
                "The only scale supported by ICmpZero uses is -1!");
-        ICmpScaledV = Rewriter.expandCodeFor(ScaledS, nullptr);
+        ICmpScaledV = Rewriter.expandCodeFor(ScaledS, Loc, nullptr);
       }
     } else {
       // Otherwise just expand the scaled register and an explicit scale,
       // which is expected to be matched as part of the address.
 
       // Flush the operand list to suppress SCEVExpander hoisting address modes.
       // Unless the addressing mode will not be folded.
       if (!Ops.empty() && LU.Kind == LSRUse::Address &&
           isAMCompletelyFolded(TTI, LU, F)) {
-        Value *FullV = Rewriter.expandCodeFor(SE.getAddExpr(Ops), Ty);
+        Value *FullV = Rewriter.expandCodeFor(SE.getAddExpr(Ops), Loc, Ty);
         Ops.clear();
         Ops.push_back(SE.getUnknown(FullV));
       }
-      ScaledS = SE.getUnknown(Rewriter.expandCodeFor(ScaledS, nullptr));
+      ScaledS = SE.getUnknown(Rewriter.expandCodeFor(ScaledS, Loc, nullptr));
       if (F.Scale != 1)
         ScaledS =
             SE.getMulExpr(ScaledS, SE.getConstant(ScaledS->getType(), F.Scale));
       Ops.push_back(ScaledS);
     }
   }
 
   // Expand the GV portion.
   if (F.BaseGV) {
     // Flush the operand list to suppress SCEVExpander hoisting.
     if (!Ops.empty()) {
-      Value *FullV = Rewriter.expandCodeFor(SE.getAddExpr(Ops), Ty);
+      Value *FullV = Rewriter.expandCodeFor(SE.getAddExpr(Ops), Loc, Ty);
       Ops.clear();
       Ops.push_back(SE.getUnknown(FullV));
     }
     Ops.push_back(SE.getUnknown(F.BaseGV));
   }
 
   // Flush the operand list to suppress SCEVExpander hoisting of both folded and
   // unfolded offsets. LSR assumes they both live next to their uses.
   if (!Ops.empty()) {
-    Value *FullV = Rewriter.expandCodeFor(SE.getAddExpr(Ops), Ty);
+    Value *FullV = Rewriter.expandCodeFor(SE.getAddExpr(Ops), Loc, Ty);
     Ops.clear();
     Ops.push_back(SE.getUnknown(FullV));
   }
 
   // Expand the immediate portion.
   int64_t Offset = (uint64_t)F.BaseOffset + LF.Offset;
   if (Offset != 0) {
     if (LU.Kind == LSRUse::ICmpZero) {
if (UnfoldedOffset != 0) {
     Ops.push_back(SE.getUnknown(ConstantInt::getSigned(IntTy,
                                                        UnfoldedOffset)));
   }
 
   // Emit instructions summing all the operands.
   const SCEV *FullS = Ops.empty() ?
                       SE.getConstant(IntTy, 0) :
                       SE.getAddExpr(Ops);
-  Value *FullV = Rewriter.expandCodeFor(FullS, Ty);
+  Value *FullV = Rewriter.expandCodeFor(FullS, Loc, Ty);
 
   // We're done expanding now, so reset the rewriter.
   Rewriter.clearPostInc();
 
   // An ICmpZero Formula represents an ICmp which we're handling as a
   // comparison against zero. Now that we've expanded an expression for that
   // form, update the ICmp's other operand.
   if (LU.Kind == LSRUse::ICmpZero) {

test/Transforms/LoopStrengthReduce/pr25630.ll

+; RUN: opt -loop-reduce < %s -S -o - | FileCheck %s
+
+; int sum(int x, int y) { return x + y; }
+;
+; int main() {
+;   int s, i = 0;
+;   while (i++ < 400)
+;     if (i != 100) s = sum(i, s); else s = 30;
+;   return s;
+; }
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.12.0"
+
+; Function Attrs: nounwind readnone ssp uwtable
+define i32 @sum(i32 %x, i32 %y) local_unnamed_addr #0 !dbg !8 {
+entry:
+  tail call void @llvm.dbg.value(metadata i32 %x, metadata !13, metadata !DIExpression()), !dbg !15
+  tail call void @llvm.dbg.value(metadata i32 %y, metadata !14, metadata !DIExpression()), !dbg !16
+  %add = add nsw i32 %y, %x, !dbg !17
+  ret i32 %add, !dbg !18
+}
+
+; Function Attrs: nounwind readnone ssp uwtable
+define i32 @main() local_unnamed_addr #0 !dbg !19 {
+entry:
+  tail call void @llvm.dbg.value(metadata i32 0, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 undef, metadata !23, metadata !DIExpression()), !dbg !26
+  tail call void @llvm.dbg.value(metadata i32 1, metadata !24, metadata !DIExpression()), !dbg !25
+  br label %while.body, !dbg !27
+
+while.body:                                       ; preds = %while.body, %entry
+  ; CHECK: [[IV:%.*]] = phi i32 [ [[IV_NEXT:%.*]], {{.*}} ], [ 5, {{.*}} ], !dbg ![[IV_LOC:[0-9]+]]
+
+  %inc8 = phi i32 [ 1, %entry ], [ %inc.4, %while.body ]
+  %s.07 = phi i32 [ undef, %entry ], [ %spec.select.4, %while.body ]
+  tail call void @llvm.dbg.value(metadata i32 %s.07, metadata !23, metadata !DIExpression()), !dbg !26
+  %cmp1 = icmp eq i32 %inc8, 100, !dbg !28
+  %add.i = add nsw i32 %inc8, %s.07, !dbg !30
+  %spec.select = select i1 %cmp1, i32 30, i32 %add.i, !dbg !32
+  tail call void @llvm.dbg.value(metadata i32 %inc8, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select, metadata !23, metadata !DIExpression()), !dbg !26
+  %inc = add nuw nsw i32 %inc8, 1, !dbg !33
+  tail call void @llvm.dbg.value(metadata i32 %inc, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select, metadata !23, metadata !DIExpression()), !dbg !26
+  %cmp1.1 = icmp eq i32 %inc, 100, !dbg !28
+  %add.i.1 = add nsw i32 %inc, %spec.select, !dbg !30
+  %spec.select.1 = select i1 %cmp1.1, i32 30, i32 %add.i.1, !dbg !32
+  tail call void @llvm.dbg.value(metadata i32 %inc, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select.1, metadata !23, metadata !DIExpression()), !dbg !26
+  %inc.1 = add nuw nsw i32 %inc8, 2, !dbg !33
+  tail call void @llvm.dbg.value(metadata i32 %inc.1, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select.1, metadata !23, metadata !DIExpression()), !dbg !26
+  %cmp1.2 = icmp eq i32 %inc.1, 100, !dbg !28
+  %add.i.2 = add nsw i32 %inc.1, %spec.select.1, !dbg !30
+  %spec.select.2 = select i1 %cmp1.2, i32 30, i32 %add.i.2, !dbg !32
+  tail call void @llvm.dbg.value(metadata i32 %inc.1, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select.2, metadata !23, metadata !DIExpression()), !dbg !26
+  %inc.2 = add nuw nsw i32 %inc8, 3, !dbg !33
+  tail call void @llvm.dbg.value(metadata i32 %inc.2, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select.2, metadata !23, metadata !DIExpression()), !dbg !26
+  %cmp1.3 = icmp eq i32 %inc.2, 100, !dbg !28
+  %add.i.3 = add nsw i32 %inc.2, %spec.select.2, !dbg !30
+  %spec.select.3 = select i1 %cmp1.3, i32 30, i32 %add.i.3, !dbg !32
+  tail call void @llvm.dbg.value(metadata i32 %inc.2, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select.3, metadata !23, metadata !DIExpression()), !dbg !26
+  %inc.3 = add nuw nsw i32 %inc8, 4, !dbg !33
+  tail call void @llvm.dbg.value(metadata i32 %inc.3, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select.3, metadata !23, metadata !DIExpression()), !dbg !26
+  %cmp1.4 = icmp eq i32 %inc.3, 100, !dbg !28
+  %add.i.4 = add nsw i32 %inc.3, %spec.select.3, !dbg !30
+  %spec.select.4 = select i1 %cmp1.4, i32 30, i32 %add.i.4, !dbg !32
+  tail call void @llvm.dbg.value(metadata i32 %inc.3, metadata !24, metadata !DIExpression()), !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %spec.select.4, metadata !23, metadata !DIExpression()), !dbg !26
+  %inc.4 = add nuw nsw i32 %inc8, 5, !dbg !33
+  tail call void @llvm.dbg.value(metadata i32 %inc.4, metadata !24, metadata !DIExpression()), !dbg !25
+  %exitcond.4 = icmp eq i32 %inc.4, 401, !dbg !34
+  br i1 %exitcond.4, label %while.end, label %while.body, !dbg !27, !llvm.loop !35
+
+; CHECK: [[IV_NEXT]] = add nuw nsw i32 [[IV]], 5, !dbg ![[IV_LOC]]
+
+while.end:                                        ; preds = %while.body
+  ret i32 %spec.select.4, !dbg !37
+}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind readnone ssp uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="penryn" "target-features"="+cx16,+fxsr,+mmx,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+; CHECK-DAG: ![[INLINE_SCOPE:[0-9]+]] = distinct !DILocation(line: 6, column: 23, scope: {{.*}})
+; CHECK-DAG: ![[IV_LOC]] = !DILocation(line: 1, column: 34, scope: !8, inlinedAt: ![[INLINE_SCOPE]])
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0 (trunk 317279) (llvm/trunk 317288)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "t.c", directory: "/Users/vk/src/llvm.org-dbginf/PR29600836-missing-inlined-at-tag")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 6.0.0 (trunk 317279) (llvm/trunk 317288)"}
+!8 = distinct !DISubprogram(name: "sum", scope: !1, file: !1, line: 1, type: !9, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !12)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !11, !11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !{!13, !14}
+!13 = !DILocalVariable(name: "x", arg: 1, scope: !8, file: !1, line: 1, type: !11)
+!14 = !DILocalVariable(name: "y", arg: 2, scope: !8, file: !1, line: 1, type: !11)
+!15 = !DILocation(line: 1, column: 13, scope: !8)
+!16 = !DILocation(line: 1, column: 20, scope: !8)
+!17 = !DILocation(line: 1, column: 34, scope: !8)
+!18 = !DILocation(line: 1, column: 25, scope: !8)
+!19 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 3, type: !20, isLocal: false, isDefinition: true, scopeLine: 3, isOptimized: true, unit: !0, variables: !22)
+!20 = !DISubroutineType(types: !21)
+!21 = !{!11}
+!22 = !{!23, !24}
+!23 = !DILocalVariable(name: "s", scope: !19, file: !1, line: 4, type: !11)
+!24 = !DILocalVariable(name: "i", scope: !19, file: !1, line: 4, type: !11)
+!25 = !DILocation(line: 4, column: 10, scope: !19)
+!26 = !DILocation(line: 4, column: 7, scope: !19)
+!27 = !DILocation(line: 5, column: 3, scope: !19)
+!28 = !DILocation(line: 6, column: 11, scope: !29)
+!29 = distinct !DILexicalBlock(scope: !19, file: !1, line: 6, column: 9)
+!30 = !DILocation(line: 1, column: 34, scope: !8, inlinedAt: !31)
+!31 = distinct !DILocation(line: 6, column: 23, scope: !29)
+!32 = !DILocation(line: 6, column: 9, scope: !19)
+!33 = !DILocation(line: 5, column: 11, scope: !19)
+!34 = !DILocation(line: 5, column: 14, scope: !19)
+!35 = distinct !{!35, !27, !36}
+!36 = !DILocation(line: 6, column: 43, scope: !19)
+!37 = !DILocation(line: 7, column: 3, scope: !19)