[PATCH] D126048: [SplitKit] Handle early clobber + tied to def correctly

[Kito Cheng via Phabricator via llvm-commits]

kito-cheng marked an inline comment as done.
kito-cheng added inline comments.


================
Comment at: llvm/lib/CodeGen/SplitKit.cpp:1355-1381
       Idx = Idx.getPrevSlot();
       if (!Edit->getParent().liveAt(Idx))
         continue;
     } else
       Idx = Idx.getRegSlot(true);
 
     SlotIndex Next = Idx.getNextSlot();
----------------
MatzeB wrote:
> Hmm I found the (existing) logic here somewhat confusing... line 1361 doing getNextSlot() just seems to counteract line 1355 moving to the previous slot, but forces line 1359 to unnaturally chose the early clobber slot.
> 
> Anyway can we streamline the code somewhat like this:
Thanks!


================
Comment at: llvm/lib/CodeGen/SplitKit.cpp:1376
+      // in [16e, 32d).
+      unsigned OpIdx = MI->findRegisterUseOperandIdx(MO.getReg());
+      unsigned DefOpIdx = MI->findTiedOperandIdx(OpIdx);
----------------
MatzeB wrote:
> Use `getOperandNo` that's faster and avoids trouble if multiple operands use the same register.
Seems like `getOperandNo` only work with iterator? and that is what we don't have here :(

Anyway I change to that, but it is because I hit a regression for a testcase from X86...so I just try to traverse all input operand see which operand is tied-def and using same register as MO here...

See CodeGen/X86/statepoint-invoke-ra.mir:
```
  dead %40:gr64 = STATEPOINT 2882400000, 0, 1, target-flags(x86-plt) @quux, $edi, 2, 0, 2, 0, 2, 6, 1, 4, %stack.0, 0, 1, 4, %stack.1, 0, 1, 4, %stack.2, 0, 1, 4, %stack.3, 0, %40:gr64, 1, 4, %stack.4, 0, 2, 1, %40:gr64(tied-def 0), 2, 0, 2, 1, 0, 0, <regmask $bh $bl $bp $bph $bpl $bx $ebp $ebx $hbp $hbx $rbp $rbx $r12 $r13 $r14 $r15 $r12b $r13b $r14b $r15b $r12bh $r13bh $r14bh $r15bh $r12d $r13d $r14d $r15d $r12w $r13w $r14w $r15w $r12wh and 3 more...>, implicit-def $rsp, implicit-def $ssp :: (volatile load store (s32) on %stack.0), (volatile load store (s32) on %stack.1), (volatile load store (s32) on %stack.2), (volatile load store (s32) on %stack.3), (volatile load store (s32) on %stack.4)
```

%40 appeared 3 times in this instruction, one for def, and two for use, and only one def has `tied-def 0`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126048