[PATCH] D104471: [llvm][sve] Lowering for VLS truncating stores

Mon Jun 28 11:22:23 PDT 2021

efriedma added a comment.

Looking again with your explanation, I understand a bit more how the pieces fit together; it just wasn't obvious at first glance.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1225
+      for (MVT InnerVT : MVT::integer_fixedlen_vector_valuetypes()) {
+        setTruncStoreAction(VT, InnerVT, Custom);
+      }
----------------
DavidTruby wrote:
> efriedma wrote:
> > Can we change this to make sure we only mark stores that we can actually lower "custom"?  (i.e. the value type is a legal integer vector, and the memory type has a legal element type.)  That should make the rest of the patch simpler, I think.
> I'm not sure what doing this would allow us to remove for the rest of the patch, could you point me at it?
Well, the change to guard LowerTruncateVectorStore is only necessary because of this, I think.  And probably it messes with DAGCombine heuristics a little.  But neither of those is a big deal, I guess.

I think it's still worth doing to illustrate the intent here, though; it's hard to someone reading this to understand why you're custom-lowering every possible truncstore.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17700
-    return SDValue();
-
   SDLoc DL(Op);
----------------
Not sure what this change is doing here; I don't see any test coverage for masked stores?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104471