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

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 28 10:47:21 PDT 2019


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





More information about the llvm-commits mailing list