Page MenuHomePhabricator

[AArch64] Support reserving x16 and x17 register
AbandonedPublic

Authored by phosek on Apr 27 2018, 3:15 PM.

Details

Summary

Registers x16 and 17 are defined as linker temporaries, which are used
for purposes such as veneers. However, in some cases these may be used
for other purposes in certain contexts, for example to hold special
variables within the kernel.

We would like to use these registers to hold the per-CPU pointer in Fuchsia
kernel. This change adds support for reserving these registers both to
frontend and backend to make these registers usable for such purpose.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.Apr 27 2018, 3:15 PM
mcgrathr accepted this revision.Apr 27 2018, 3:31 PM

I don't know the backend or the register allocator at all but this looks like a thorough parallel of the existing model for -ffixed-x18.

llvm/lib/Target/AArch64/AArch64Subtarget.cpp
155 ↗(On Diff #144406)

We could imagine dying default reserved-x16 or reserved-x17 to -mcmodel=kernel (as we do with TIPIDR_EL1 e.g.) but that's probably not worthwhile since we want the flexibility to change what we're using in the kernel.

llvm/test/CodeGen/AArch64/arm64-platform-reg.ll
25

Possibly worth separately testing combinations of reserve-{x16,x17,x18}. It would be a bug if max register pressure used any when more than one is enabled.

This revision is now accepted and ready to land.Apr 27 2018, 3:31 PM
phosek updated this revision to Diff 144427.Apr 27 2018, 6:48 PM
phosek marked an inline comment as done.

I have some doubts this is the right long term approach.

As https://android.googlesource.com/kernel/common/+/c0385b24af15020a1e505f2c984db0d7c0d017e1%5E%21/#F4 indicates, there are others who have use cases for reserving other registers than x16,x17,x18. (The linked patch reserves x1,x2,x3,x4,x5,x6,x7).
With that indication, I think it may be better to bite the bullet now and implement full support for gcc command line option "-ffixed-reg", where reg is, according to the gcc documentation: "reg must be the name of a register. The register names accepted are machine-specific and are defined in the REGISTER_NAMES macro in the machine description macro file.".
For the full documentation of this option, see https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html.

What do you think?

Thanks,

Kristof

I have some doubts this is the right long term approach.

As https://android.googlesource.com/kernel/common/+/c0385b24af15020a1e505f2c984db0d7c0d017e1%5E%21/#F4 indicates, there are others who have use cases for reserving other registers than x16,x17,x18. (The linked patch reserves x1,x2,x3,x4,x5,x6,x7).
With that indication, I think it may be better to bite the bullet now and implement full support for gcc command line option "-ffixed-reg", where reg is, according to the gcc documentation: "reg must be the name of a register. The register names accepted are machine-specific and are defined in the REGISTER_NAMES macro in the machine description macro file.".
For the full documentation of this option, see https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html.

What do you think?

Thanks,

Kristof

That was original proposal bug @chandlerc suggested in our offline discussion that it's not easily doable. It seems like we might be able to duplicate the same code for all remaining registers which will require a lot of duplicated logic but it should work? However, I might be missing some details of the register allocator which will break.

I have some doubts this is the right long term approach.

As https://android.googlesource.com/kernel/common/+/c0385b24af15020a1e505f2c984db0d7c0d017e1%5E%21/#F4 indicates, there are others who have use cases for reserving other registers than x16,x17,x18. (The linked patch reserves x1,x2,x3,x4,x5,x6,x7).
With that indication, I think it may be better to bite the bullet now and implement full support for gcc command line option "-ffixed-reg", where reg is, according to the gcc documentation: "reg must be the name of a register. The register names accepted are machine-specific and are defined in the REGISTER_NAMES macro in the machine description macro file.".
For the full documentation of this option, see https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html.

What do you think?

Thanks,

Kristof

That was original proposal bug @chandlerc suggested in our offline discussion that it's not easily doable. It seems like we might be able to duplicate the same code for all remaining registers which will require a lot of duplicated logic but it should work? However, I might be missing some details of the register allocator which will break.

My hope was that the most non-trivial part would be to replicate the gcc implementation detail described as "The register names accepted are machine-specific and are defined in the REGISTER_NAMES macro in the machine description macro file." in the documentation.

But let's see if @qcolombet or @MatzeB know any reason why there would be more complexities to support the generic gcc command line option.

phosek abandoned this revision.Jun 11 2018, 8:15 PM

We're going to use D46552 instead.