[PATCH] D140796: [ValueTracking] Improve ComputeNumSignBits to handle Trunc

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:3334
+
       break;
+    }
----------------
resistor wrote:
> nikic wrote:
> > return 1 here, we don't want to fall through to computeKnownBits (the ComptueNumSignBits will have already used it if possible).
> I don't think that's the case? We invoked `ComputeNumSignBits` on the input to the trunc, so we haven't yet had an opportunity to use `computeKnownBits` results for the output.
Hm, I guess you are right. If the truncated part doesn't have sign bits and the part directly following has more than one known zero/one bits, then the computeKnownBits() call is still useful.

Do we already have a test that fails if it is removed?


================
Comment at: llvm/test/Transforms/InstCombine/vector-trunc.ll:24
+;
+  %2 = ashr <4 x i32> %0, <i32 8, i32 8, i32 8, i32 8>
+  %3 = trunc <4 x i32> %2 to <4 x i16>
----------------
It would be slightly better to test the values where the behavior flips, so I guess that should be a shift by 17 above and by 16 here.


================
Comment at: llvm/test/Transforms/InstCombine/vector-trunc.ll:41
+  ret <4 x i16> %4
+}
----------------
Am I missing something, or is this test the same as the previous one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140796