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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:4940
+  DebugLoc Loc;
+  if (auto *I = dyn_cast<Instruction>(LF.OperandValToReplace))
+    Loc = I->getDebugLoc();
----------------
What besides an instruction could this be? An Argument?


================
Comment at: test/Transforms/LoopStrengthReduce/pr25630.ll:57
+  %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
----------------
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


================
Comment at: test/Transforms/LoopStrengthReduce/pr25630.ll:89
+
+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 }
----------------
we usually strip out all non-essential attributes from tests


https://reviews.llvm.org/D39986