[PATCH] D148316: [AArch64] Add support for efficient bitcast in vector truncate store.

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 20 10:00:26 PDT 2023


dmgreen added a comment.

In D148316#4280756 <https://reviews.llvm.org/D148316#4280756>, @lawben wrote:

> I dug into this a bit, and your vector compare sing bit patch <https://reviews.llvm.org/D148624> does the right thing here. But the problem is unrelated and a bit annoying. At the time the `sign_extend_inreg` that we added is combined (to potentially remove it), there is a `build_vector` with `(1 << vectorElementSize) - 1` bits (to negate via `xor`). But `ComputeNumSignBits` breaks here. The vector has 32-bit constants but we only have 8-bit vectors. So it detects 24 sign bits and then kinda gives up. So without changing code in `SelectionDAG::ComputeNumSignBits` or the negation of vector entries, there is no way to correctly determine the sign bits.
>
> If you have any ideas how to overcome this, please let me know. Otherwise, I'd leave this in `performSTORECombine` to apply it before legalization (`vec != 0` --> `(vec == 0) ^ 255` ) breaks it.

Yeah that sounds OK. There might be a way to fix it by teaching something to truncate constants properly, but many of those things can lead to more and more issues that need to be accounted for. Having this as a combine sounds like it should be fine, they just needs to more careful about illegal types and it looks like those should be accounted for.

I think it can remove the setTruncStoreAction's if it goes through a combine?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148316



More information about the llvm-commits mailing list