[PATCH] D11649: [regalloc] Make RegMask clobbers prevent merging vreg's into PhysRegs when hoisting def's upwards.

[Daniel Sanders]

dsanders added a comment.

That's right, the vreg260 and http://reviews.llvm.org/D7 can't be coalesced because the intervening JAL instruction clobbers http://reviews.llvm.org/D7 (it's not in the call-preserved regmask). Coalescing them would hoist the def of http://reviews.llvm.org/D7 (for the copy) above another def of http://reviews.llvm.org/D7 (for the calls clobber).

The relevant snippet of the original test is (almabench.c) is:

  for (k = 8; k <= 9; ++k) {
      argl = kq[np][k] * dmu;
      dl   = dl + t * ( cl[np][k] * cos(argl) + sl[np][k] * sin(argl) ) * 0.0000001;
  }
  
  dl = fmod(dl,TWOPI);

Occasionally the TWOPI constant is zero instead of 2*PI. What's happening is the ldc1 instruction (load to FPU register) that loads the TWOPI constant into $f14 (known as http://reviews.llvm.org/D7 internally and is the second FPU argument register) is being moved by the coalescer from just before the fmod call, to a point just above three sin/cos calls from the unrolled loop. However, $f14 is call-clobbered and sometimes the sin/cos calls clobber the value.


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:1541
@@ -1534,1 +1540,3 @@
+          return false;
+      }
     }
----------------
qcolombet wrote:
> This check should have been handled by the loop starting line 1497, IIRC.
> I.e., this shouldn’t be necessary.
I agree that that loop should have caught this but it doesn't. I haven't looked into why that loop doesn't catch it yet but if the information is based on MachineInstr::definesRegister() then it could be that that doesn't seem to account for regmasks.

I also tried:
    if (MI->definesRegister(DstReg, TRI)) {
      DEBUG(dbgs() << "\t\tInterference (read): " << *MI);
      return false;
    }
in this loop but that doesn't catch it either.


http://reviews.llvm.org/D11649