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

[ShihPo Hung via Phabricator via llvm-commits]

arcbbb added inline comments.


================
Comment at: llvm/lib/CodeGen/SplitKit.cpp:1381
+        unsigned OpIdx = MI->getOperandNo(MOItr);
+        unsigned DefOpIdx = MI->findTiedOperandIdx(OpIdx);
+        const MachineOperand &DefOp = MI->getOperand(DefOpIdx);
----------------
kito-cheng wrote:
> arcbbb wrote:
> > The logic here looks similar to LiveIntervalCalc::extendToUses that checks for early-clobber redefs.
> > Can it be simplified with `OpIdx = (&MO - &MI->getOperand(0))` and `MI->isRegTiedToDefOperand(OpIdx, &DefOpIdx)`?
> `MI->getOperandNo(MOItr)` did almost similar thing[1] as `(&MO - &MI->getOperand(0))`, but having better readability.
> 
> And `isRegTiedToDefOperand` called `findTiedOperandIdx` in the function implementation[2], so I am not sure it's a simplification?
> 
> [1] https://llvm.org/doxygen/MachineInstr_8h_source.html#l00685
> [2] https://llvm.org/doxygen/MachineInstr_8h_source.html#l00685
I think it can save the `for` loop and do checks for you
so the code would look like
```lang=diff
-    } else
-      Idx = Idx.getRegSlot(true);
+    } else {
+      unsigned OpNo = MI->getOperandNo(&MO);
+      unsigned DefIdx;
+      bool isEarlyClobber = false;
+      if (MI->isRegTiedToDefOperand(OpNo, &DefIdx))
+        isEarlyClobber = MI->getOperand(DefIdx).isEarlyClobber();
+      Idx = Idx.getRegSlot(isEarlyClobber);
+    }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126048