This is an archive of the discontinued LLVM Phabricator instance.

Start reserving x18 by default on Android targets.
ClosedPublic

Authored by pcc on Apr 12 2018, 1:16 PM.

Diff Detail

Repository
rC Clang

Event Timeline

pcc created this revision.Apr 12 2018, 1:16 PM
kcc added a comment.Apr 12 2018, 5:18 PM

Don't you need a test?
srhines@, what kind of documentation change need to be done?

pcc updated this revision to Diff 142321.Apr 12 2018, 8:07 PM
  • Add test
pcc added a comment.Apr 12 2018, 8:07 PM
In D45588#1066620, @kcc wrote:

Don't you need a test?

Probably the easiest way to test this is to check that clang -fsanitize=shadow-call-stack does not complain about needing -ffixed-x18, so that is what I've done.

kcc added a comment.Apr 13 2018, 11:07 AM

This test is great, but I'd like to also see some more direct test, maybe something in tools/clang/test/Driver/aarch64-fixed-x18.c?

LGTM for the code, but I still want to hear LGTM from srhines@

pcc updated this revision to Diff 142448.Apr 13 2018, 11:32 AM
  • Add LLVM code generation test
pcc added a comment.Apr 13 2018, 11:34 AM
In D45588#1067402, @kcc wrote:

This test is great, but I'd like to also see some more direct test, maybe something in tools/clang/test/Driver/aarch64-fixed-x18.c?

That won't test what we want here because the platform defaults are not applied via target features. I added a case for Android to one of the code generation tests.

kcc added a comment.Apr 13 2018, 11:35 AM

Yep, that's the one, thanks!

srhines accepted this revision.Aug 28 2018, 6:00 PM

Sorry, this got buried in my inbox back in April right before EuroLLVM.

This revision is now accepted and ready to land.Aug 28 2018, 6:00 PM
This revision was automatically updated to reflect the committed changes.