[PATCH] D48331: [DebugInfo][InstCombine] Preserve DI after merging instructions

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1099
+        auto Fragment = DIExpression::createFragmentExpression(
+            DII->getExpression(), SrcBitsKept, DestBitSize);
+        DIB.insertDbgValueIntrinsic(
----------------
vsk wrote:
> vsk wrote:
> > + @aprantl and @probinson for dwarf expertise.
> > 
> > There are two cases: a zext can either operate on integers or on vectors of integers. I think the fragment you're creating here is correct in the first case, but not the second. For the second case, I think you'd to emit multiple dbg.values with distinct fragments describing each component of the original vector.
> With the helper I suggested, the zext-of-integer case could be simplified to:
> 
> ```
> InsertReplacementDebugValues(Src, Res, &*std::next(CI.getIterator()), [&](DbgInfoIntrinsic &OldDII) -> DIExpression * {
>   return DIExpression::createFragmentExpression(OldDII.getExpression(), SrcBitsKept, DestBitSize);
> });
> ```
Emitting one fragment per element sounds plausible, but Adrian is really the fragment expert.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1092
+    // Since the old instruction is merged, we preserve it's DI as
+    // a fragment in the resulting instruction
+    SmallVector<DbgInfoIntrinsic *, 1> SrcDbgInsts;
----------------
Comment should end with a full-stop. Also `it's` should be `its` here.


Repository:
  rL LLVM

https://reviews.llvm.org/D48331