[PATCH] D135324: [AArch64-SVE]: force using SVE in streaming mode

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

Hi @hassnaa-arm, it looks like this patch is based off D133433 <https://reviews.llvm.org/D133433>. Can you add that as a parent revision so it's obvious to the reviewer please? You can do this by clicking on "Edit Related Revisions" -> "Edit Parent Revisions" at the top-right corner of the page. Thanks!



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4460
+  if (VT.isScalableVector() ||
+      useSVEForFixedLengthVectorVT(VT, Subtarget->forceSVEInStreamingMode()))
     return LowerToPredicatedOp(Op, DAG, AArch64ISD::MUL_PRED);
----------------
Hi @hassnaa-arm, this change doesn't look right. I would expect it to break some tests? When we're not in streaming mode we also want to override NEON for 64-bit element types. Can you put the OverrideNEON flag back in, perhaps something like


  // If SVE is available then i64 vector multiplications can also be made legal.
  bool OverrideNEON = VT == MVT::v2i64 || VT == MVT::v1i64 || Subtarget->forceSVEInStreamingMode();


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-int-arith.ll:91
+; VBITS_GE_128-NEXT:    add z1.b, z1.b, z5.b
+; VBITS_GE_128-NEXT:    stp q0, q1, [x0, #32]
+; VBITS_GE_128-NEXT:    add z0.b, z3.b, z7.b
----------------
Again, this is illegal in streaming mode.


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-int-arith.ll:1587
+; VBITS_GE_128-NEXT:    abs z1.h, p0/m, z2.h
+; VBITS_GE_128-NEXT:    stp q0, q1, [x0]
+; VBITS_GE_128-NEXT:    ret
----------------
This looks like a NEON instruction - can you investigate where this is coming from?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135324