This is an archive of the discontinued LLVM Phabricator instance.

[COFF, ARM64] Reserve X18 register by default
ClosedPublic

Authored by mgrang on Jul 18 2017, 12:34 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

mgrang created this revision.Jul 18 2017, 12:34 AM
mstorsjo added inline comments.Jul 18 2017, 3:40 AM
lib/Target/AArch64/AArch64Subtarget.cpp
176 ↗(On Diff #107022)

The TT.isOSBinFormatCOFF() part feels a little superfluous here

test/CodeGen/AArch64/arm64-platform-reg.ll
1 ↗(On Diff #107022)

Unrelated to this particular patch, but shouldn't we drop the -mattr=+reserve-x18 here, to also test that darwin actuall does this right by default? (I'm not sure if there's any other tests that explicitly test that?)

28 ↗(On Diff #107022)

Why the custom test prefix for windows/coff? Doesn't the existing CHECK-RESERVE-X18 also work for aarch64-windows?

mgrang added inline comments.Jul 18 2017, 11:06 AM
lib/Target/AArch64/AArch64Subtarget.cpp
176 ↗(On Diff #107022)

True. It can be removed.

test/CodeGen/AArch64/arm64-platform-reg.ll
1 ↗(On Diff #107022)

I thought so too. Will fix this in another patch.

28 ↗(On Diff #107022)

I wasn't sure about the "ldr fp" and "Spill". Is fp an alias for x18?

mstorsjo added inline comments.Jul 18 2017, 12:11 PM
test/CodeGen/AArch64/arm64-platform-reg.ll
28 ↗(On Diff #107022)

fp is an alias for x29; I think that part of the test just makes sure that nothing tries to use that register for the copying that the test does (especially when x18 isn't available), since that register shouldn't be used for unrelated things.

mgrang updated this revision to Diff 107159.Jul 18 2017, 12:45 PM

Removed isOSBinFormatCOFF().
Removed extra check label CHECK-RESERVE-X18-COFF.

mgrang marked 2 inline comments as done.Jul 18 2017, 12:46 PM
mstorsjo accepted this revision.Jul 18 2017, 12:46 PM
This revision is now accepted and ready to land.Jul 18 2017, 12:46 PM
This revision was automatically updated to reflect the committed changes.