This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Define subtarget feature "reserve-x18"
ClosedPublic

Authored by ahatanak on Jul 23 2015, 8:16 AM.

Details

Summary

This patch defines a subtarget feature "reserve-x18" and uses it to decide whether X18 should be reserved.

The clang-side patch which makes changes to pass a subtarget feature instead of a backend option is here: http://reviews.llvm.org/D11462

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak updated this revision to Diff 30488.Jul 23 2015, 8:16 AM
ahatanak retitled this revision from to [AArch64] Define subtarget feature "reserve-x18" .
ahatanak updated this object.
ahatanak added reviewers: echristo, dexonsmith.
ahatanak added a subscriber: llvm-commits.
echristo edited edge metadata.Jul 23 2015, 1:50 PM
echristo added a subscriber: echristo.

So you're defining it to true here and only have an option to set it to
true from the front end.

How would you turn it off? :)

-eric

Currently, there is no option to turn off reserve-x18 for Darwin (x18 is always reserved for Darwin), but I'm assuming it isn't there because no one needs it?

Then why the subtarget option and the command line option? Theoretically
it's off for someone?

-eric

According to the original email, the option was added because they wanted to reserve x18 for free bsd kernel but not didn't want to do so for userland programs. They wanted an option to reserve register x18, which is normally not reserved.

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150119/121916.html

Right, but from looking at the two patches basically it's on all the time
right? You initialize it to true and the only way to pass anything is to
turn it on.

ahatanak updated this revision to Diff 30528.Jul 23 2015, 3:26 PM
ahatanak edited edge metadata.

I noticed ReserveX18 wasn't being initialized in AArch64Subtarget. ReserveX18 is now initialized to false and becomes true only when subtarget feature "+reserve-x18" is passed. Sorry for the confusion.

emaste added a subscriber: emaste.Jul 24 2015, 8:38 AM
echristo accepted this revision.Jul 24 2015, 10:35 AM
echristo edited edge metadata.

Inline comment/question. Can be done as a followup if you'd like.

Thanks!

-eric

lib/Target/AArch64/AArch64RegisterInfo.cpp
402–403 ↗(On Diff #30528)

I wonder if the isOSDarwin parts here necessarily need to be here - should be able to fold them into either a) the subtarget feature existing, or b) the query in TFI?

This revision is now accepted and ready to land.Jul 24 2015, 10:35 AM
This revision was automatically updated to reflect the committed changes.
ahatanak added inline comments.Jul 27 2015, 10:02 AM
lib/Target/AArch64/AArch64RegisterInfo.cpp
402–403 ↗(On Diff #30528)

Yes, either the front-end should pass reserve-x18 if the target is darwin or we should have a function in subtarget or TFI that returns (isOSDDarwin() || isX18Reserved()).