Specify a platform reserved register #370
Comments
|
It would be little bit later to reserve a reserved register for RISC-V since the ABI already used for a while, so adding one general reserved register would be annoying to the current ecosystem especially for the linux distro guys... However here is an evil thought in my mind, GP relaxation might not very useful on non-baremetal environment so we might use that as special use for fuchsia and Android other than GP relaxation, that come with one more advantage is: all existing code base didn't use |
|
Android can have a different ABI that reserves a register--perhaps |
|
As I understand it, this issue is not a request to change the Linux ABI, but to solicit recommendations on minimally disruptive specializations of it. It is understood that this would result in a new, incompatible ABI. Aside from the technical question, there is also the procedural question of how/where to standardize this. I think my suggestion to @appujee to jump-start the discussion here was misguided, and it would be better to start at sig-toolchains. FWIW, I think the Android folks would be perfectly happy with a nonnormative note in the psABI --- e.g., "variant ABIs that add a platform register are encouraged to throw away GP-relaxation and use |
|
Please quantitatively evaluate the value of the |
|
https://reviews.llvm.org/D84414 is the review that contains the x18 choice; see my various comments there on this issue. I agree x18 is entirely unprincipled and wish there had been more willingness to think carefully about what register made sense for SCS. |
sgtm, i'll start the discussion on sig-toolchain.
Yes. It is better to have something that one can point to. |
agreed with your concerns there. Let me start a discussion on sig-toolchain to address this. |
|
I've proposed a change in LLVM doing this, since it would be better to do sooner rather than later. Thankfully the changes are minimal, and there are not likely to be too many users depending of RISCV SCS. https://reviews.llvm.org/D146463 |
|
FYI: Anders Berg mentioned in https://lists.riscv.org/g/sig-toolchains/topic/97736679#544 :
|
That work has been promised for over a year at this point; blocking changing the shadow call stack ABI on that is not reasonable in my opinion, especially given it is already implemented and in use, just with a potentially bad choice of register. If and when the EABI deviations/whatever it's called these days finally turns up it can be adopted for annotating such binaries to prevent incompatible objects being mixed. |
|
makes sense. I've approved the patch https://reviews.llvm.org/D146463 by @ilovepi. Do review when you can. |
|
Oh I wasn't sure if Anders was referring to the EABI stuff or not. If so, I'm not completely sure it's relevant to this request, but it's hard to tell until there's a full proposal to review. I agree there shouldn't be too many SCS users and it would be good to change sooner rather than later. I think we should make a little more effort to check for downstream users who may be affected though. |
|
Well, the EABI stuff has become the same as the ABI deviations stuff, so yes and no. |
|
#371 created for the possibility of using GP, that's also discussed during the EABI/ABI deviations stuffs, so I think sooner or later we will need that. |
|
We do not want to slow this process down, because it is important that we come to a resolution as soon as possible, but we do want to raise these concerns now, while there is still time to take them into account. Ideally, projects that place a premium on code size wouldn’t have to forgo code size savings from global relaxation if they want to use SCS, or make use of the platform register. We see no fundamental reason why users in the embedded space who care deeply about code size would not fall into this category, and it would be nice if they could also make use of the feature without being required to drop global relaxation. As mentioned in the sig-toolchain meeting and the ps ABI issues, there are many benefits to using gp, but before we make a choice we should be sure that we are explicit about the tradeoffs are why we believe they are worth making. |
|
@ilovepi thanks for the comment, SCS is kind of special is that eventually will break the ABI (for RISC-V) since that require one extra reserved register, so that give we few more freedom to having more option than other ABI issues. Of cause the why it become an issue that must did a ABI breakage change is we didn't specify a platform register at beginning, but anyway the boat sailed. So that's back to the SCS first, I think we have three options:
Pick a GPR as SCSThere are actually two candidate during the discussion x18 and x27, but x18 is kind of many potential issues, so now we are discussion the other candidate, and then x27 has purposed.
but it's still has low probability might screwed up in some asm code since it was not reserved before, so I am not prefer to pick up a non-reserved register if possible. Re-define gp to allow that use other than gp relaxation.Already listed several reason in #371, so not duplicate here, so just jump to the concern @ilovepi raised, what if user want SCS and gp relaxation, I think it's the most potential drawback of this proposal.
What's the possible solution if people want more code size saving AND SCS?
NOTE: Use
|
| Option 1 | Option 2 | |
|---|---|---|
| Used in existing asm code? | Yes *1 | No |
| Impact code size | Yes, but very minor | Yes *2 |
| Break ABI | Hard break | Soft break compare to option 1 |
So based on the comparison table and several reason listed in #371, I believe we should go with option 2
*1: google/android-riscv64#78 Android have an issue to tracked where has use the SCS register candidate.
*2: We have some compiler optimization to make up the gap like GlobalMerge optimization, and be noted there is no loss on those system already disable GP relaxation.
|
@kito-cheng , Thank you for the summary. I'm pretty confident at this point that we will adopt As you mention, there may be compiler side work we can do to ameliorate the situation to some extent(i.e., GlobalMerge in LLVM or its GCC equivalent). I think that's probably something we all may want to investigate more: how our toolchains can close any remaining gaps for the tradeoffs we make in the ABI. |
|
Note that if the register gp is reserved as a platform register in the platform-independent ABI, it is still possible to reserve one additional platform register in a platform ABI specification. In other words, reserving gp does not preclude also reserving s11 in a platform ABI specification. |
ShadowCallStack implementation uses s2 register on RISC-V, but that choice is problematic for reasons described in: https://lists.riscv.org/g/sig-toolchains/message/544, riscv-non-isa/riscv-elf-psabi-doc#370, and google/android-riscv64#72 The concern over the register choice was also brought up in https://reviews.llvm.org/D84414. https://reviews.llvm.org/D84414#2228666 said: ``` "If the register choice is the only concern about this work, then I think we can probably land it as-is and fixup the register choice if we see major drawbacks later. Yes, it's an ABI issue, but on the other hand the shadow call stack is not a standard ABI anyway."" ``` Since we have now found a sufficient reason to fixup the register choice, we should go ahead and update the implementation. We propose using x3(gp) which is now the platform register in the RISC-V ABI. Reviewed By: asb, hiraditya, mcgrathr, craig.topper Differential Revision: https://reviews.llvm.org/D146463
|
Closed via #371 |
ShadowCallStack implementation uses s2 register on RISC-V, but that choice is problematic for reasons described in: https://lists.riscv.org/g/sig-toolchains/message/544, riscv-non-isa/riscv-elf-psabi-doc#370, and google/android-riscv64#72 The concern over the register choice was also brought up in https://reviews.llvm.org/D84414. https://reviews.llvm.org/D84414#2228666 said: ``` "If the register choice is the only concern about this work, then I think we can probably land it as-is and fixup the register choice if we see major drawbacks later. Yes, it's an ABI issue, but on the other hand the shadow call stack is not a standard ABI anyway."" ``` Since we have now found a sufficient reason to fixup the register choice, we should go ahead and update the implementation. We propose using x3(gp) which is now the platform register in the RISC-V ABI. Reviewed By: asb, hiraditya, mcgrathr, craig.topper Differential Revision: https://reviews.llvm.org/D146463
ShadowCallStack implementation uses s2 register on RISC-V, but that choice is problematic for reasons described in: https://lists.riscv.org/g/sig-toolchains/message/544, riscv-non-isa/riscv-elf-psabi-doc#370, and google/android-riscv64#72 The concern over the register choice was also brought up in https://reviews.llvm.org/D84414. https://reviews.llvm.org/D84414#2228666 said: ``` "If the register choice is the only concern about this work, then I think we can probably land it as-is and fixup the register choice if we see major drawbacks later. Yes, it's an ABI issue, but on the other hand the shadow call stack is not a standard ABI anyway."" ``` Since we have now found a sufficient reason to fixup the register choice, we should go ahead and update the implementation. We propose using x3(gp) which is now the platform register in the RISC-V ABI. Reviewed By: asb, hiraditya, mcgrathr, craig.topper Differential Revision: https://reviews.llvm.org/D146463
ARM reserves x18 for platform but RISC-v doesn't specify a platform register.
https://developer.arm.com/documentation/den0024/a/The-ABI-for-ARM-64-bit-Architecture/Register-use-in-the-AArch64-Procedure-Call-Standard/Parameters-in-general-purpose-registers
Recent changes in llvm has reserved the same
x18for Android and Fuschia platforms to align with the name w.r.t. ARM spec. But x18 may not be an ideal choice for RISC-V because:-msave-restoredoes not interact well ifx18gets reserved.Some discussion has been conitnuing here: google/android-riscv64#72 but it would be ideal to have a spec or at least a recommendation on choice of platform registers and trade-offs around that
In the current setup
x27appears to be a better choice as this would reduce:-msave-restoreX18is used in more places (3rd callee saved register) than X27.The text was updated successfully, but these errors were encountered: