[arm] Support reserving r4 and r5 alongside r9 #89849
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: None (benisxdxd) ChangesI accidentally closed this pr when trying to fix a conflict that arose. Full diff: https://github.com/llvm/llvm-project/pull/89849.diff 6 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 922bda721dc780..3a110dbbec9690 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4766,6 +4766,10 @@ def mrestrict_it: Flag<["-"], "mrestrict-it">, Group<m_arm_Features_Group>,
def mno_restrict_it: Flag<["-"], "mno-restrict-it">, Group<m_arm_Features_Group>,
HelpText<"Allow generation of complex IT blocks.">;
def marm : Flag<["-"], "marm">, Alias<mno_thumb>;
+def ffixed_r4 : Flag<["-"], "ffixed-r4">, Group<m_arm_Features_Group>,
+ HelpText<"Reserve the r4 register (ARM only)">;
+def ffixed_r5 : Flag<["-"], "ffixed-r5">, Group<m_arm_Features_Group>,
+ HelpText<"Reserve the r5 register (ARM only)">;
def ffixed_r9 : Flag<["-"], "ffixed-r9">, Group<m_arm_Features_Group>,
HelpText<"Reserve the r9 register (ARM only)">;
def mno_movt : Flag<["-"], "mno-movt">, Group<m_arm_Features_Group>,
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index a68368c4758651..f29a01f6e364ab 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -920,6 +920,11 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
if (Args.hasArg(options::OPT_ffixed_r9))
Features.push_back("+reserve-r9");
+ if (Args.hasArg(options::OPT_ffixed_r4))
+ Features.push_back("+reserve-r4");
+ if (Args.hasArg(options::OPT_ffixed_r5))
+ Features.push_back("+reserve-r5");
+
// The kext linker doesn't know how to deal with movw/movt.
if (KernelOrKext || Args.hasArg(options::OPT_mno_movt))
Features.push_back("+no-movt");
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
index 9adf758b46c481..6ebf7b4c706c89 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -211,9 +211,13 @@ getReservedRegs(const MachineFunction &MF) const {
markSuperRegs(Reserved, STI.getFramePointerReg());
if (hasBasePointer(MF))
markSuperRegs(Reserved, BasePtr);
- // Some targets reserve R9.
+ // Some targets reserve R4,R5 or R9.
if (STI.isR9Reserved())
markSuperRegs(Reserved, ARM::R9);
+ if (STI.isReserveR4())
+ markSuperRegs(Reserved, ARM::R4);
+ if (STI.isReserveR5())
+ markSuperRegs(Reserved, ARM::R5);
// Reserve D16-D31 if the subtarget doesn't support them.
if (!STI.hasD32()) {
static_assert(ARM::D31 == ARM::D16 + 15, "Register list not consecutive!");
diff --git a/llvm/lib/Target/ARM/ARMFeatures.td b/llvm/lib/Target/ARM/ARMFeatures.td
index 111c87838291f6..587cf0aea0b091 100644
--- a/llvm/lib/Target/ARM/ARMFeatures.td
+++ b/llvm/lib/Target/ARM/ARMFeatures.td
@@ -458,6 +458,14 @@ def FeatureExecuteOnly : SubtargetFeature<"execute-only",
def FeatureReserveR9 : SubtargetFeature<"reserve-r9", "ReserveR9", "true",
"Reserve R9, making it unavailable"
" as GPR">;
+// True if R4 is not available as a general purpose register.
+def FeatureReserveR4 : SubtargetFeature<"reserve-r4", "ReserveR4", "true",
+ "Reserve R4, making it unavailable"
+ " as GPR">;
+// True if R5 is not available as a general purpose register.
+def FeatureReserveR5 : SubtargetFeature<"reserve-r5", "ReserveR5", "true",
+ "Reserve R5, making it unavailable"
+ " as GPR">;
// True if MOVT / MOVW pairs are not used for materialization of
// 32-bit imms (including global addresses).
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.h b/llvm/lib/Target/ARM/ARMSubtarget.h
index 497ae160fde281..3ea6fd9425137c 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.h
+++ b/llvm/lib/Target/ARM/ARMSubtarget.h
@@ -437,6 +437,14 @@ class ARMSubtarget : public ARMGenSubtargetInfo {
bool isRClass() const { return ARMProcClass == RClass; }
bool isAClass() const { return ARMProcClass == AClass; }
+ bool isR4Reserved() const {
+ return ReserveR4;
+ }
+
+ bool isR5Reserved() const {
+ return ReserveR5;
+ }
+
bool isR9Reserved() const {
return isTargetMachO() ? (ReserveR9 || !HasV6Ops) : ReserveR9;
}
diff --git a/llvm/lib/Target/ARM/ARMTargetTransformInfo.h b/llvm/lib/Target/ARM/ARMTargetTransformInfo.h
index 04b32194f806f6..e7376f4f71abc4 100644
--- a/llvm/lib/Target/ARM/ARMTargetTransformInfo.h
+++ b/llvm/lib/Target/ARM/ARMTargetTransformInfo.h
@@ -92,8 +92,9 @@ class ARMTTIImpl : public BasicTTIImplBase<ARMTTIImpl> {
ARM::FeatureHasNoBranchPredictor, ARM::FeatureDSP, ARM::FeatureMP,
ARM::FeatureVirtualization, ARM::FeatureMClass, ARM::FeatureRClass,
ARM::FeatureAClass, ARM::FeatureNaClTrap, ARM::FeatureStrictAlign,
- ARM::FeatureLongCalls, ARM::FeatureExecuteOnly, ARM::FeatureReserveR9,
- ARM::FeatureNoMovt, ARM::FeatureNoNegativeImmediates
+ ARM::FeatureLongCalls, ARM::FeatureExecuteOnly, ARM::FeatureReserveR4,
+ ARM::FeatureReserveR5, ARM::FeatureReserveR9, ARM::FeatureNoMovt,
+ ARM::FeatureNoNegativeImmediates
};
const ARMSubtarget *getST() const { return ST; }
|
|
Ping |
There was a problem hiding this comment.
There was a previous attempt at doing something similar with more global registers r6-r11 in https://reviews.llvm.org/D68862 based on http://lists.llvm.org/pipermail/llvm-dev/2018-December/128706.html
This got reverted, and unfortunately didn't get picked back up.
One of the comments in the description of the patch is:
Additionally this patch now only supports r6-r11. r4 and r5 are excluded from this patch as r4 is used as hard-coded scratch register in various parts of the ARM backend. r4 also appears to be used as an input register for a Windows asm routine (__chkstk). Similarly, the ABI of the segmented stack prologue for Android and Linux seems to use r4 and r5 as input registers. A separate patch could follow to add the support for r4 and/or r5, such that the whole range of allocatable registers (r4-r11) is available.
My suggestions:
- Describe why R4 and R5 specifically and not R6 - R11?
- If R4 and R5 are required then show that the hard-coded use in the backend is no longer there. If R4 and R5 are not required I suggest picking up the changes in https://reviews.llvm.org/D68862
- Add tests, especially if registers that have other uses in procedure call standards, or frame chains are used.
- Given that the original was reverted due to interactions with r6, explain what further tests that you've made such as compiling large bodies of code or fuzz tested against programs.
|
@smithp35 The system i'm interacting with has a GCC plugin that uses r4 and r5 for custom software mitigations against exploitation. |
|
@smithp35 But I don't think it will cause an issue because it is basically and intrinsic and is restored right afterwards. |
I accidentally closed this pr when trying to fix a conflict that arose.