[PATCH] D36795: [SystemZ] Increase number of LOCRs emitted by passing regalloc hints

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa updated this revision to Diff 111636.
jonpa added a comment.

> The getRegAllocationHints implementation makes sense to me. However, I'm wondering if we shouldn't also check for the *destination* register -- you only force one source register to the same class as the other source register, but I think we should check whether *any* of the three registers is already allocated, and then always force the other two to the same class.

I tried this (see updated patch below), but found that it gave the identical results (with or without the DestMO lines which I commented out). My guess is that the the TwoAddress pass is already doing this job in processTiedPairs() by

  const TargetRegisterClass *RC = MRI->getRegClass(RegB);
  ...
      if (TargetRegisterInfo::isVirtualRegister(RegA) &&
          TargetRegisterInfo::isVirtualRegister(RegB))
        MRI->constrainRegClass(RegA, RC);



> I think if we do that consistently, we should be able to *guarantee* that we end up with a register assignment that is legal for the instruction, and can therefore completely remove the pass that creates a branch again.

Giving hints is not enough if VirtReg is GRX32, and there could still be cases where the source registers are high / low. The question is how/when should we constrain the reg classes? It doesn't seem right to do this in getRegAllocationHints(), since e.g. already Order is passed. It could be done in emitSelect(), but a bit naively (complicated cases suboptimally). It would be nice to do this after coalescing, but not sure how.

> Already constraining the the regclass in emitSelect may make sense as well, but we should verify that it still makes any difference once we've implemented the change above.

I don't know exactly why, but doing only this (without hints) give some improved benchmarks without regressions. Doing only the hints give similar improvements, but with regressions... Also, a regression with the regsplit around loops patch applied could only be handled with this, so this part seems valuable until we actually manage to handle the same cases with hints.

> I agree that the LOCRXALTERNATE variant doesn't look very useful.

removed


https://reviews.llvm.org/D36795

Files:
  lib/Target/SystemZ/SystemZISelLowering.cpp
  lib/Target/SystemZ/SystemZRegisterInfo.cpp
  lib/Target/SystemZ/SystemZRegisterInfo.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36795.111636.patch
Type: text/x-patch
Size: 5653 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170818/57df433b/attachment.bin>