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