[llvm] r243308 - [AArch64] Remove check for Darwin that was needed to decide if x18 should
Akira Hatanaka via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 13 12:29:21 PST 2015
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.
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?
On Fri, Nov 13, 2015 at 10:53 AM, Justin Bogner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> (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
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151113/cb7c8207/attachment.html>
More information about the llvm-commits
mailing list