[PATCH] D138682: [AArch64][SME]: Generate streaming-compatible code for bit counting/select

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15694
   // it does not work for SVE when dealing with vectors wider than 128 bits.
-  if (!VT.is64BitVector() && !VT.is128BitVector())
+  if (!VT.is64BitVector() && !VT.is128BitVector() ||
+      DAG.getSubtarget<AArch64Subtarget>().forceStreamingCompatibleSVE())
----------------
mgabka wrote:
> david-arm wrote:
> > I think you need brackets around the first part, i.e.
> > 
> >   if ((!VT.is64BitVector() && !VT.is128BitVector()) ||
> I think this code is introducing a bug, before the combine had an early exit if the VT wasn't 64 or 128 bit vector, while now we allow scalable vectors, and the combine does not give correct results for scalable vectors, it is just not triggered because the "ISD::isBuildVectorAllZeros" and "ISD::isBuildVectorAllOnes" reject vector splat, but I think it would be better to have a clear early exit for scalable VT.
> What do you think? 
>From what I can see, `useSVEForFixedLengthVectorVT` returns `false` if `VT` is a scalable vector, so I don't think this would be a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138682