[PATCH] D80615: [CodeGen] Fix warnings in getPackedVectorTypeFromPredicateType

[David Sherwood via Phabricator via llvm-commits]

david-arm marked an inline comment as done.
david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:4626-4629
+  ElementCount EC = PredVT.getVectorElementCount();
 
-  if (NumElts != 2 && NumElts != 4 && NumElts != 8 && NumElts != 16)
+  if (EC.Min != 2 && EC.Min != 4 && EC.Min != 8 && EC.Min != 16)
     return EVT();
----------------
fpetrogalli wrote:
> NIT:
> 
> We could be explicit here on the PredVT that we want to use and swap the statements as follows:
> 
> ```
> if (PredVT != MVT::nxv16i1 && PredVT != MVT::nxv8i1 && ...)
>   return EVT();
> 
> ElementCount EC = ...
> ```
> 
> I am also wondering whether we can remove at all the idea of using EC.Min in this code and just use TypeSize. Something like the following:
> 
> ```
> if (PredVT != MVT::nxv16i1 && PredVT != MVT::nxv8i1 && ...)
>   return EVT();
> 
> // This could actually be a global in the AArch64 namespace, to be used as AArch64::SVEBits,
> // I suspect it will be handy in other situations.
> const auto SVEVecSize = TypeSize::Scalable(AArch64::SVEBitsPerBlock);
> 
> EVT MemVT = PredVT;
> while (MemVT.getSizeInBits() < SVETySy)
>   MemVT = MemVT.widenIntegerVectorElementType(Ctx);
> 
> return MemVT;
> ```
> 
> Seems like a good thing to get rid of the EC.Min here?
I think changing your first suggestion for checking the PredVT makes sense, however I don't think we should be afraid of using EC.Min here for calculations. However, I think your second suggestion is inefficient, since we are already in the AArch64 backend and we know exactly how to deal with scalable vectors. I believe calculating (SVEBitsPerBlock / EC.Min) is the right thing to do here - we know at this point EC is for a legal type, since we've already discarded illegal types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80615