[PATCH][regalloc] Fix the interference calculation for register masks.
qcolombet at apple.com
Wed Nov 13 09:57:37 PST 2013
On Nov 13, 2013, at 9:24 AM, Andrew Trick <atrick at apple.com> wrote:
> On Nov 13, 2013, at 8:54 AM, Jakob Stoklund Olesen <jolesen at apple.com> wrote:
>> On Nov 12, 2013, at 9:14 PM, Andrew Trick <atrick at apple.com> wrote:
>>> On Nov 12, 2013, at 8:30 PM, Juergen Ributzka <juergen at apple.com> wrote:
>>>> Hi @ll,
>>>> this patch fixes a false interference for register masks.
>>>> Register masks are defined in the normal use/def register slot and therefore any register defined in the register mask should not interfere with a register assigned to the result of the same instruction.
>>>> This hasn’t been a problem until now, because register make are used by call instructions and they don't have an explicit def. The patchpoint intrinsic may have an explicit def and the resulting code has unnecessary register copy instructions. This was due to the false interference with the register mask.
>>>> This fixes <rdar://problem/15440848>
>>> Thanks Juergen,
>>> This isn’t the only place we check regmask interference. There’s also RAGreedy::tryLocalSplit. Let’s keep them consistent. Also, I think we should add a comment explaining why a regmask at the calls ‘r’ slot does not interfere with a live range defined at the same ‘r’ slot.
>>> I don’t see a better way to do this, but copying Jakob just to be sure it’s the right fix.
>> A register mask operand is supposed to be just a shortcut notation for a set of dead defs, with the same semantics. You seem to be introducing a different liveness model for register masks here that I don't quite understand. Can you clarify?
>> If you're actually changing the liveness model, I'm a bit skeptical that you can pull it off by tweaking a couple inequalities.
> If an instruction has a normal (r slot) def biased toward %rax and a dead def of %rax, can the normal def be assigned to %rax?
> The situation is an intrinsic call returning its value in a vreg.
> %vreg0 = PATCHPOINT regmask(~%rax, …)
> %rax = copy %vreg0
> The way I see it, the regmask clobbers exist somewhere between the use’s ‘r’ slot and the def’s ‘r’ slot. We don’t have a slot for that :)
I agree with both the definition and the fact we are lacking this semantic.
To be honest, before Jakob clarification, I thought the regmask has the between uses and defs semantic and that dead-defs were explicitly modeled with dead register definitions.
I am surprised we haven’t seen that problem before, because calls should have the exact same problem for return value. Indeed, return values are both defined and clobbered by calls. I guess we did not see that because for calls the definitions were physical registers, thus no need to allocate them.
Sorry if I sound like captain obvious.
> However, by just tweaking the inequalities, I worry that we could cause interference within a bundle.
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits