This is an archive of the discontinued LLVM Phabricator instance.

Pass subtarget feature "+reserve-r9" instead of passing backend option "-arm-reserve-r9"
ClosedPublic

Authored by ahatanak on Jul 17 2015, 5:15 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak updated this revision to Diff 30054.Jul 17 2015, 5:15 PM
ahatanak retitled this revision from to Pass subtarget feature "+reserve-r9" instead of passing backend option "-arm-reserve-r9".
ahatanak updated this object.
ahatanak added reviewers: echristo, dexonsmith.
ahatanak added a subscriber: cfe-commits.
t.p.northover accepted this revision.Jul 20 2015, 11:38 AM
t.p.northover added a reviewer: t.p.northover.
t.p.northover added a subscriber: t.p.northover.

Looks reasonable (obviously assuming it goes in at the same time as D11320).

Tim.

This revision is now accepted and ready to land.Jul 20 2015, 11:38 AM
echristo edited edge metadata.Jul 20 2015, 11:45 AM

Question:

Would it be better to just use reserve=NUM and parse that rather than the single +/- per register? Or is it just not worth it here?

Question:

Would it be better to just use reserve=NUM and parse that rather than the single +/- per register? Or is it just not worth it here?

I guess it could be useful for debugging or testing code-gen passes (for example, register allocator).

AArch64 has a similar option "aarch64-reserve-x18". Are there other targets or use cases that require reserving registers?

The only other use case I can think of is something like the -ffixed-reg= option. I remember there being some issues with trying that in the register allocator, but it might make sense.

Thoughts?

-eric

MatzeB added a subscriber: MatzeB.Jul 20 2015, 2:05 PM

This case sounds like r9 is a typical ABI difference on the ARM target. Unless there is an example of a real-world ARM ABI case that would benefit from reserving another register I'd not introduce a general reservation mechanism.

In any case this should be independent of switches to the test the register allocator. Testing switches should use cl::opts while proper ABI stuff should indeed use subtarget features.

echristo accepted this revision.Jul 20 2015, 2:17 PM
echristo edited edge metadata.

Sure, I'm willing to go with that.

Thanks!

-eric

Thanks, I'll go with this patch then.

This revision was automatically updated to reflect the committed changes.