[PATCH] D85364: [SVE][WIP] Implement lowering for fixed width select

Cameron McInally via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 3 08:40:35 PDT 2020


cameron.mcinally added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7248
     if (SrcVT.getSizeInBits() < VT.getSizeInBits()) {
       assert(2 * SrcVT.getSizeInBits() == VT.getSizeInBits());
       // We can pad out the smaller vector for free, so if it's part of a
----------------
efriedma wrote:
> Is this assert also a potential issue?
Not sure. The fixed-width lowering case should never get here. Splitting will only be taking a subvector out of a larger vector with the same element type.
 


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7286
 
       Src.ShuffleVec = DAG.getNode(AArch64ISD::EXT, dl, DestVT, VEXTSrc1,
                                    VEXTSrc2,
----------------
efriedma wrote:
> Is AArch64ISD::EXT going to work with arbitrarily large vectors?
The fixed-width lowering case should never get here. We always have one source that's being split.

But, yeah, I agree that this is a little sketchy. Maybe we should have a separate routine to lower the splitting case? I don't know. Just thinking out loud.

Or maybe we should have a special 1 source case within ReconstructShuffle(...)? That could work.


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

https://reviews.llvm.org/D85364



More information about the llvm-commits mailing list