[PATCH] D73711: [AArch64][SVE] Add support for DestructiveBinary and DestructiveBinaryComm DestructiveInstTypes

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen accepted this revision.
sdesmalen added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:167
+    switch(N->getOpcode()) {
+    case ISD::BUILD_VECTOR: {
+      auto Splat = cast<BuildVectorSDNode>(N)->getSplatValue();
----------------
cameron.mcinally wrote:
> cameron.mcinally wrote:
> > sdesmalen wrote:
> > > BUILD_VECTOR only works for fixed-width vectors, so you'll need to check for SPLAT_VECTOR instead.
> > > 
> > > This also puzzles me a bit on how the tests could pass; I'd either expect the pattern not to match because the incoming value is not a BUILD_VECTOR, or I'd expect an assertion in BUILD_VECTOR to fail because it doesn't work with scalable vectors.
> > Hmm, that's strange. I assumed there was a separate missing lowering somewhere for zeroinitializer. 
> > 
> > I've definitely seen other DestructiveInstr types matching DUP/SPLAT there. Maybe I missed something subtle in this patch. 
> > 
> > Will check it out...
> Ah, got it. ISel is missing support for FP splat_vectors.
> 
> I'll do that under a separate Diff, assuming I can find a good way to add standalone tests for the splat_vectors...
Thanks for fixing this and adding the support for splat_vectors in the other patch!


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:176
+          return true;
+    }
+    default:
----------------
nit: missing either `break;` or otherwise `LLVM_FALLTHROUGH` statement here.


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

https://reviews.llvm.org/D73711