[PATCH] D66295: [ARM] Sink add/mul(shufflevector(insertelement(...), ...), ...) for MVE instruction selection

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:14325
+      if (Shuffle) {
+        for (User* User : Shuffle->users()) {
+          Instruction* Insn = dyn_cast<Instruction>(User);
----------------
Can you add a comment saying that we want all uses to be sunk (else we'll just end up with the same value in both gpr and vector regs!)

Also if this loop is moved to after the match check below, we won't need to do the O(n) loop if the O(1) match fails. Also negating the condition and returning early is apparently a good thing in llvm.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:14327
+          Instruction* Insn = dyn_cast<Instruction>(User);
+          if (!IsSinker(Insn, Op))
+            return false;
----------------
I think this should be checking the operand of the use, which may not be the same for the original instruction.


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

https://reviews.llvm.org/D66295