[PATCH] D108115: [DAG][sve] Lowering for VLS masked truncating stores

[Dave Green via Phabricator via llvm-commits]

dmgreen added reviewers: craig.topper, RKSimon.
dmgreen added a comment.

Sorry for the delay. I missed this as something I should be looking at.

I've had a look around, and I can't see anything obvious that goes wrong with this. canCombineTruncStore is a bit overloaded between stores and masked stores - it relies on the support being symetrical.

> I'm nervous about splitting the DAG combine into a separate patch because it's currently only triggered by SVE code generation; adding it in a separate patch leaves it unused and untested in that patch which I'd rather try and avoid.

OK Fair enough. MVE at least takes a separate route to turning masked stores to truncating stores, but has less legal types to deal with. We can treat the MVE test that improved between revisions as the tests.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9828
+
+    // See if we can simplify the operation with
+    // SimplifyDemandedBits, which only works if the value has a single use.
----------------
This formatting looks off.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18161
 
+  if (Store->isTruncatingStore()) {
+    Mask = DAG.getNode(
----------------
Why do we need to extend the mask? So that convertFixedMaskToScalableVector shrinks to a i1 vector again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108115