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

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1093
+            auto DIExp = OldDII.getExpression();
+            if (DIExp) {
+              return DIExpression::createFragmentExpression(
----------------
DIExpression doesn't need to be null-checked here because DbgValueInst::getExpression() is always nonnull.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1094
+            if (DIExp) {
+              return DIExpression::createFragmentExpression(
+                         DIExp, DestBitSize - SrcBitsKept,
----------------
You mentioned offline that this causes a verifier failure. If we can't use fragments, consider using a simpler approach for scalars: using an empty expression. It won't work for vectors, but in the scalar case, the high bits should already be cleared. We can relax the check in https://reviews.llvm.org/D48408 to allow this.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1097
+                         SrcTy->getScalarSizeInBits())
+                  .getValue();
+            }
----------------
This unwraps an Optional without a check. If you're sure this can't fail, please add a comment explaining why.


Repository:
  rL LLVM

https://reviews.llvm.org/D48331