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

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

In https://reviews.llvm.org/D36795#845175, @jonpa wrote:

> > 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


Ah, of course.  I had forgotten that these are two-operand instructions anyway, so my suggestion doesn't actually make sense.  (I must have been thinking of three-operand instructions like AHHHR.)

>> 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.

So my thought that if both registers are still GRX32, then getRegAllocationHints for the first register would return both high and low options.  Then, once regalloc chooses one of them, and getRegAllocationHints is later called on the *other* register, we know it must be in the same class, so we return only the low or only the high options.  Does it not work this way?

>> 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.

OK.  In any case, constraining the regclass in emitSelect, in cases where we already know about restrictions, seems a good idea.


https://reviews.llvm.org/D36795