[PATCH] D58466: [AArch64][GlobalISel] Implement partial support for G_SHUFFLE_VECTOR

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1935
+  // Find the constant indices.
+  for (unsigned i = 1; i < MaskDef->getNumOperands(); ++i) {
+    MachineInstr *ScalarDef = MRI.getVRegDef(MaskDef->getOperand(i).getReg());
----------------
`unsigned i = 1, e = MaskDef->getNumOperands(); i < e; ++i`?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1939-1941
+    while (ScalarDef->getOpcode() == TargetOpcode::COPY) {
+      ScalarDef = MRI.getVRegDef(ScalarDef->getOperand(1).getReg());
+    }
----------------
don't need braces


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1940
+    while (ScalarDef->getOpcode() == TargetOpcode::COPY) {
+      ScalarDef = MRI.getVRegDef(ScalarDef->getOperand(1).getReg());
+    }
----------------
`MRI.getVRegDef` isn't guaranteed to return a value. Could this possibly be null here?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1995-1996
+  // G_SHUFFLE_VECTOR is weird in that the source operands can be scalars, if
+  // it's originated from a <1 x T> type. Those should have been custom
+  // legalized into something else before we get here.
+  if (!Src1Ty.isVector() || !Src2Ty.isVector())
----------------
Should we die here then? Or add some debug output to make it easier to grep logs for if it happens?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58466