[PATCH] D124834: [fastregalloc] Fix bug when undef value is tied to def.

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1115-1122
+  auto IsTiedDef = [&](unsigned Idx) {
+    MachineOperand &MO = MI.getOperand(Idx);
+    if (!MO.isTied())
+      return false;
+    unsigned TiedIdx = MI.findTiedOperandIdx(Idx);
+    auto TiedMO = MI.getOperand(TiedIdx);
+    return !TiedMO.isUndef();
----------------
arsenm wrote:
> Might as well pass in MachineOperand since that's what you already have at the use points 
I think this would be easier to understand for readers if you go for a different name and semantic. Despite the use being "undef" it's still a tied operand after all. You just want to change the algorithm to ignore those, but for that I think it's clearer to state that within the algorithm.

So how about:


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1130-1142
+  for (unsigned I = 0; I < MI.getNumOperands(); ++I) {
+    MachineOperand &MO = MI.getOperand(I);
     if (MO.isReg()) {
       Register Reg = MO.getReg();
       if (Reg.isVirtual()) {
         if (MO.isDef()) {
           HasDef = true;
----------------
In the above spirit change this to:


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1242
           unsigned Reg = MO.getReg();
-          if (MO.isEarlyClobber() || MO.isTied() ||
+          if (MO.isEarlyClobber() || IsTiedDef(OpIdx) ||
               (MO.getSubReg() && !MO.isUndef())) {
----------------



================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1281
       // Do not free tied operands and early clobbers.
-      if (MO.isTied() || MO.isEarlyClobber())
+      if (IsTiedDef(I) || MO.isEarlyClobber())
         continue;
----------------



================
Comment at: llvm/test/CodeGen/X86/fastregalloc-tied-undef.mir:28-51
+name:            foo
+alignment:       16
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: vr128 }
+frameInfo:
+  maxAlignment:    16
----------------
On top of dropping the IR you can probably simplify like this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124834