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

[Lawrence Benson via Phabricator via llvm-commits]

lawben added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1223
+    setTruncStoreAction(MVT::v2i64, MVT::v2i1, Custom);
+    setTruncStoreAction(MVT::v8i8, MVT::v16i1, Custom);
+    setTruncStoreAction(MVT::v4i16, MVT::v4i1, Custom);
----------------
dmgreen wrote:
> v8i1? It doesn't appear that these are necessary though, as the trunc stores are already created and the new code goes through a combine.
> 
> You could alternatively add the call to combineVectorCompareAndTruncateStore to lowerSTORE instead of the combine.
regarding moving it to `lowerSTORE`, before I go down that path and change the code. i originally had it in there and then moved it to the "combine" pass, because the truncating store is legalized _after_ the `setcc`. so we have lots of compare logic with intermediate `XOR` etc., which hinders current other passes to fully optimize them out again. we are essentially calling truncate, then sign extend. the sign extend is removed if it knows that `numSignBits` is equal to the element's size. but we lose this information after legalization . if i have this logic applied before legalization, we don't run into that issue. 

I'm happy to move the code and figure our how to get the optimizations to apply again if `lowerSTORE` is the correct location. if this was just a suggestion, i think leaving it in the combine pass is better. just wanted to get your opinion before implementing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148316