[PATCH] D58684: [AArch64][GlobalISel] Add support for 64 bit vector shuffle using TBL1

[Amara Emerson via Phabricator via llvm-commits]

aemerson marked 2 inline comments as done.
aemerson added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1979
+      SubregIdx = AArch64::ssub;
+    } else {
+      Opc = AArch64::INSvi64gpr;
----------------
paquette wrote:
> Is it possible that `EltSize` could be something invalid when it's coming into here? Should we have some error-handling?
Yeah I can add llvm_unreachable() to catch the unexpected cases. We shouldn't be seeing anything else here I think.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1981
+      Opc = AArch64::INSvi64gpr;
+      SubregIdx = AArch64::dsub;
+    }
----------------
paquette wrote:
> Not important for this patch, but I've noticed that we have a lot of this subregister calculation code floating around.
> 
> Might be good to migrate some of this over to using a utility function for calculating the subregister index. `getSubRegForClass` does it, but I don't know if that's the best choice.
Yes I considered using that, but it was expecting a RegClass (and also has a special case for gpr32). I agree we'll want to have some common internal API to get this information in future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58684