[llvm] r243308 - [AArch64] Remove check for Darwin that was needed to decide if x18 should
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 13 10:53:39 PST 2015
(resending with the correct list. Sorry for the duplicate)
Akira Hatanaka <ahatanaka at apple.com> writes:
> Author: ahatanak
> Date: Mon Jul 27 14:18:47 2015
> New Revision: 243308
>
> URL: http://llvm.org/viewvc/llvm-project?rev=243308&view=rev
> Log:
> [AArch64] Remove check for Darwin that was needed to decide if x18 should
> be reserved.
>
> The decision to reserve x18 is going to be made solely by the front-end,
> so it isn't necessary to check if the OS is Darwin in the backend.
Sorry for digging up such an old thread, but I don't think this is the
right way to do this. With this change, anyone other than clang (like a
JIT) will generate ABI-incompatible code on Darwin by default.
Basically, this makes it so that any code generator for Darwin has to
set the +reserve-x18 feature to even be correct.
I think the right thing to do is to default ReserveX18 in the
AArch64Subtarget constructor based on isOSDarwin. ie:
diff --git a/lib/Target/AArch64/AArch64Subtarget.cpp b/lib/Target/AArch64/AArch64Subtarget.cpp
index 88af960..14ad44d 100644
--- a/lib/Target/AArch64/AArch64Subtarget.cpp
+++ b/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -53,8 +53,9 @@ AArch64Subtarget::AArch64Subtarget(const Triple &TT, const std::string &CPU,
: AArch64GenSubtargetInfo(TT, CPU, FS), ARMProcFamily(Others),
HasV8_1aOps(false), HasFPARMv8(false), HasNEON(false), HasCrypto(false),
HasCRC(false), HasPerfMon(false), HasZeroCycleRegMove(false),
- HasZeroCycleZeroing(false), StrictAlign(false), ReserveX18(false),
- IsLittle(LittleEndian), CPUString(CPU), TargetTriple(TT), FrameLowering(),
+ HasZeroCycleZeroing(false), StrictAlign(false),
+ ReserveX18(TT.isOSDarwin()), IsLittle(LittleEndian), CPUString(CPU),
+ TargetTriple(TT), FrameLowering(),
InstrInfo(initializeSubtargetDependencies(FS)), TSInfo(),
TLInfo(TM, *this) {}
If we do this we can also revert r243310 since it'll become redundant. WDYT?
> Modified:
> llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp
> llvm/trunk/test/CodeGen/AArch64/arm64-platform-reg.ll
> llvm/trunk/test/CodeGen/AArch64/arm64-stackmap.ll
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp?rev=243308&r1=243307&r2=243308&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp Mon Jul 27 14:18:47 2015
> @@ -100,7 +100,7 @@ AArch64RegisterInfo::getReservedRegs(con
> Reserved.set(AArch64::W29);
> }
>
> - if (TT.isOSDarwin() || MF.getSubtarget<AArch64Subtarget>().isX18Reserved()) {
> + if (MF.getSubtarget<AArch64Subtarget>().isX18Reserved()) {
> Reserved.set(AArch64::X18); // Platform register
> Reserved.set(AArch64::W18);
> }
> @@ -127,8 +127,7 @@ bool AArch64RegisterInfo::isReservedReg(
> return true;
> case AArch64::X18:
> case AArch64::W18:
> - return TT.isOSDarwin() ||
> - MF.getSubtarget<AArch64Subtarget>().isX18Reserved();
> + return MF.getSubtarget<AArch64Subtarget>().isX18Reserved();
> case AArch64::FP:
> case AArch64::W29:
> return TFI->hasFP(MF) || TT.isOSDarwin();
> @@ -398,12 +397,11 @@ unsigned AArch64RegisterInfo::getRegPres
> case AArch64::GPR64RegClassID:
> case AArch64::GPR32commonRegClassID:
> case AArch64::GPR64commonRegClassID:
> - return 32 - 1 // XZR/SP
> - - (TFI->hasFP(MF) || TT.isOSDarwin()) // FP
> - - (TT.isOSDarwin() ||
> - MF.getSubtarget<AArch64Subtarget>()
> - .isX18Reserved()) // X18 reserved as platform register
> - - hasBasePointer(MF); // X19
> + return 32 - 1 // XZR/SP
> + - (TFI->hasFP(MF) || TT.isOSDarwin()) // FP
> + - MF.getSubtarget<AArch64Subtarget>()
> + .isX18Reserved() // X18 reserved as platform register
> + - hasBasePointer(MF); // X19
> case AArch64::FPR8RegClassID:
> case AArch64::FPR16RegClassID:
> case AArch64::FPR32RegClassID:
>
> Modified: llvm/trunk/test/CodeGen/AArch64/arm64-platform-reg.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-platform-reg.ll?rev=243308&r1=243307&r2=243308&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/arm64-platform-reg.ll (original)
> +++ llvm/trunk/test/CodeGen/AArch64/arm64-platform-reg.ll Mon Jul 27 14:18:47 2015
> @@ -1,4 +1,4 @@
> -; RUN: llc -mtriple=arm64-apple-ios -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE-X18
> +; RUN: llc -mtriple=arm64-apple-ios -mattr=+reserve-x18 -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE-X18
> ; RUN: llc -mtriple=arm64-freebsd-gnu -mattr=+reserve-x18 -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE-X18
> ; RUN: llc -mtriple=arm64-linux-gnu -o - %s | FileCheck %s
>
>
> Modified: llvm/trunk/test/CodeGen/AArch64/arm64-stackmap.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-stackmap.ll?rev=243308&r1=243307&r2=243308&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/arm64-stackmap.ll (original)
> +++ llvm/trunk/test/CodeGen/AArch64/arm64-stackmap.ll Mon Jul 27 14:18:47 2015
> @@ -1,5 +1,5 @@
> -; RUN: llc -mtriple=arm64-apple-darwin < %s | FileCheck %s
> -; RUN: llc -mtriple=arm64-apple-darwin -fast-isel -fast-isel-abort=1 < %s | FileCheck %s
> +; RUN: llc -mtriple=arm64-apple-darwin -mattr=+reserve-x18 < %s | FileCheck %s
> +; RUN: llc -mtriple=arm64-apple-darwin -mattr=+reserve-x18 -fast-isel -fast-isel-abort=1 < %s | FileCheck %s
> ;
> ; Note: Print verbose stackmaps using -debug-only=stackmaps.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list