[AArch64] Support reserving x1-7 registers.

[trong]

Summary

Reserving registers x1-7 is used to support CONFIG_ARM64_LSE_ATOMICS in Linux kernel. This change adds support for reserving registers x1 through x7.

[srhines]

test/CodeGen/AArch64/arm64-platform-reg.ll

16 ↗

(On Diff #152825)

It would probably be good to test that multiple/all of these options work together as well.

[phosek]

lib/Target/AArch64/AArch64.td

75 ↗

(On Diff #152825)

Since you're already making this change, can we support reserving all 32 registers?

lib/Target/AArch64/AArch64RegisterInfo.cpp

413 ↗

(On Diff #152825)

Just a nit, but this should be NumReserved.

lib/Target/AArch64/AArch64Subtarget.h

135 ↗

(On Diff #152825)

Can we make this a BitVector?

[efriedma]

x1-x7 are argument registers in the calling convention; what's supposed to happen if there's a call in the code?

I can see two possibilities:

1. We emit an error.
2. We change the calling convention so it doesn't use the reserved register.

Your patch implements neither of these choices; instead, it will just miscompile.

[srhines]

In D48580#1144224, @efriedma wrote:

x1-x7 are argument registers in the calling convention; what's supposed to happen if there's a call in the code?

I can see two possibilities:

1. We emit an error.
2. We change the calling convention so it doesn't use the reserved register.

Your patch implements neither of these choices; instead, it will just miscompile.

Changing the calling convention seems like it would be potentially surprising, so I would think that emitting an error is the preferred case here. This feature is being used in areas of the Linux kernel already, and is mostly intended for experts to fine-tune their own calling conventions in some performance-sensitive areas.

[kristof.beyls]

I believe this is aiming to implement the -ffixed-reg command line option from gcc.
The documentation is at https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html:

-ffixed-reg
Treat the register named reg as a fixed register; generated code should never refer to it (except perhaps as a stack pointer, frame pointer or in some other fixed role).

reg must be the name of a register. The register names accepted are machine-specific and are defined in the REGISTER_NAMES macro in the machine description macro file.

This flag does not have a negative form, because it specifies a three-way choice.

A few thoughts:

1. From the documentation and a few related StackOverflow questions I get the impression that in the gcc implementation, the programmer is supposed to understand what he's doing when using this option. Not sure if gcc provides any error messages or in exactly which circumstances it will error. My hand wavy feel is that it'd be nice to at least warn when something seems incorrect - but given this is a command line option that seems to be used by "users who know best", the compiler giving an error may prevent the user from doing something he understands and needs to be done and cannot be achieved in any other way?

2. It seems this command line option was discussed years ago too: http://lists.llvm.org/pipermail/llvm-dev/2012-October/054033.html. I think Chris's point about LTO compilation may be especially relevant. Shouldn't this be implemented as a function attribute to make sure this won't break in LTO builds?

3. I assume we'll end up supporting this feature for multiple/all targets. A target-independent way to support this would be nice. My guess is that incrementally introducing this with only AArch64 support to start with would be fine, as long as there is an idea of what the path forward is to make this feature available for all targets.

4. How hard would it be to generalize this to be able to specify reserving any register, not just the X and (implicitly) W registers, automatically derived from the register info available in lib/Target/AArch64/AArch64RegisterInfo.td?

[efriedma]

Shouldn't this be implemented as a function attribute to make sure this won't break in LTO builds?

You can express target features as function attributes: `"target-features"="+reserve-x1" etc. Maybe worth adding a testcase to show that works.

the compiler giving an error may prevent the user from doing something he understands and needs to be done and cannot be achieved in any other way

I'm mostly worried we'll end up in situations where the option appears to work, but then miscompiles with a newer compiler, or different optimization options. For example, the code you explicitly wrote doesn't pass arguments in x7, but argpromotion increases the number of arguments to a function. Or your code doesn't pass arguments in x2, but -Os makes a struct assignment lower to a call to memcpy instead of expanding the copy inline. Or a user tries to reserve x16, we outline some code, and the linker inserts a stub that clobbers x16.

[kristof.beyls]

In D48580#1145509, @efriedma wrote:

the compiler giving an error may prevent the user from doing something he understands and needs to be done and cannot be achieved in any other way

I'm mostly worried we'll end up in situations where the option appears to work, but then miscompiles with a newer compiler, or different optimization options. For example, the code you explicitly wrote doesn't pass arguments in x7, but argpromotion increases the number of arguments to a function. Or your code doesn't pass arguments in x2, but -Os makes a struct assignment lower to a call to memcpy instead of expanding the copy inline. Or a user tries to reserve x16, we outline some code, and the linker inserts a stub that clobbers x16.

That makes a lot of sense to me. I'm not sure how easy it would be to implement such an error/warning/diagnostic without needing to add some custom logic to a lot of passes. Did you happen to have any thoughts on what a feasible way might be to implement that?

[efriedma]

For call argument registers, we only assign arguments to registers in one place, so it's easy to emit an error message if the IR contains a call which would require a reserved register. The tricky part would be avoiding spurious error messages. There are two potential sources of spurious error messages: calls to builtin functions (compiler-rt builtins/memcpy/etc.), and IPO passes which rewrite the calling convention.

For IPO passes, we could disallow rewriting the signature of functions using the C calling convention (so only fastcc functions would get rewritten), and change the calling convention so fastcc functions never use reserved registers. Only one problem with this: fastcc calls where the caller/callee have different target attributes currently misbehave (see also https://bugs.llvm.org/show_bug.cgi?id=37358 .)

For the builtin functions, not sure what the right solution would be; it gets awkward fast. If we're expecting that users will only use these for small amounts of code, we might be able to get away with just avoiding calls to builtins in common cases.

In practice, for the kernel's use-case, I think it's unlikely it would actually trigger either of these issues...? But hard to be sure.

(It's basically impossible to reliably reserve x16 or x17 given the linker constraints.)

---

Looking into it a bit more, I'm not sure -ffixed-x1 is actually what the kernel wants. They don't actually need to reserve the registers, just make them callee-save. clang has an attribute which can do this in a much more targeted way; see https://clang.llvm.org/docs/AttributeReference.html#preserve-all-clang-preserve-all-clang-preserve-all .

[manojgupta]

@efriedma Maybe not relevant to the patch here but our kernel devs were looking into preserve_all but it does not seem to work for AArch64.

$ cat test.c
void attribute((preserve_all)) foo(void *ptr) { }

$ clang -c test.c -> compiles for x86_64
$ clang -c -target aarch64-unknown-linux-gnu

fatal error: error in backend: Unsupported calling convention.

[jyknight]

I'm generally opposed to supporting the -ffixed- -fcall-used- and -fcall-saved- options from GCC. I think they're almost never the correct answer to a problem someone has.

In particular, here, it does seem that a calling convention annotation on the functions would be a significantly better way of spelling this.

[jyknight]

In D48580#1168470, @manojgupta wrote:

@efriedma Maybe not relevant to the patch here but our kernel devs were looking into preserve_all but it does not seem to work for AArch64.

$ cat test.c
void attribute((preserve_all)) foo(void *ptr) { }

$ clang -c test.c -> compiles for x86_64
$ clang -c -target aarch64-unknown-linux-gnu

fatal error: error in backend: Unsupported calling convention.

What clang version is that? Works fine for me.

[manojgupta]

What clang version is that? Works fine for me.

I tested this on the current trunk.

[jyknight]

In D48580#1168490, @jyknight wrote:

In D48580#1168470, @manojgupta wrote:

@efriedma Maybe not relevant to the patch here but our kernel devs were looking into preserve_all but it does not seem to work for AArch64.

$ cat test.c
void attribute((preserve_all)) foo(void *ptr) { }

$ clang -c test.c -> compiles for x86_64
$ clang -c -target aarch64-unknown-linux-gnu

fatal error: error in backend: Unsupported calling convention.

What clang version is that? Works fine for me.

Ugh, nevermind -- I take that back. *Old* clang didn't complain about it, new clang does. Haven't checked, maybe it was ignoring it before or something. :(

Anyhow, IMO, we should make that work, and not do this.

[trong]

In D48580#1168502, @jyknight wrote:

In D48580#1168490, @jyknight wrote:

In D48580#1168470, @manojgupta wrote:

@efriedma Maybe not relevant to the patch here but our kernel devs were looking into preserve_all but it does not seem to work for AArch64.

$ cat test.c
void attribute((preserve_all)) foo(void *ptr) { }

$ clang -c test.c -> compiles for x86_64
$ clang -c -target aarch64-unknown-linux-gnu

fatal error: error in backend: Unsupported calling convention.

What clang version is that? Works fine for me.

Ugh, nevermind -- I take that back. *Old* clang didn't complain about it, new clang does. Haven't checked, maybe it was ignoring it before or something. :(

Anyhow, IMO, we should make that work, and not do this.

It does seem like the kernel only needs registers to be callee-saved for a small number of functions. I agree that calling convention annotation is a nicer solution.
I'll investigate further if we can support CONFIG_ARM64_LSE_ATOMICS with function attributes instead of -ffixed-, etc.

[trong]

Making registers callee-saved for CONFIG_ARM64_LSE_ATOMICS regresses kernel performance since it results in extra save/restores of argument registers. So for performance it's preferable to use -ffixed-x[1-7].

GCC implementation of -ffixed does not alter the calling convention, i.e. function calls use x0-x7 as argument registers regardless of -ffixed- flags. The flags prevent allocation of those registers. Linux kernel code relies on this behavior, e.g. https://github.com/torvalds/linux/blob/master/arch/arm64/lib/Makefile#L14. atomic_ll_sc.o is built with -ffixed-x[1-7], but its callers are not.

IIUC, miscompilation can happen if caller and callee have different sets of reserved registers. In this case we should emit a warning (not an error) since the user is expected handle such cases, e.g. kernel code https://github.com/torvalds/linux/blob/master/arch/arm64/include/asm/atomic_ll_sc.h

As efriedma pointed out, clang can (already does?) express -ffixed- flags as function attributes `"target-features"="+reserve-x1". So it should be possible to issue appropriate warnings during link time.

Kernel maintainers prefer if clang supported -ffixed flags for theirs use case. https://www.spinics.net/lists/arm-kernel/msg671434.html

Is it reasonable to implement -ffixed-x[0-7] flags for argument registers without changing calling convention and emitting warnings when caller/callee have different set of reserved registers? Afterwards, we could generalize to other aarch64 GPRs. Then we could implement a more general attribute, which I imagine shouldn't be too difficult since IR already has a representation for reserved registers.

[efriedma]

IIUC, miscompilation can happen if caller and callee have different sets of reserved registers.

This is not the problem, at all. The problem is simply ensuring that the option does something sane. gcc effectively ignores the option in some cases. For example, consider the following with -ffixed-x1:

struct S { int x[100]; };
struct S x, y;
void f() { x=y; }

gcc generates the instruction "add x1, x3, :lo12:y", which violates the request to reserve x1. With the current version of the patch, LLVM will do something similar.

So we have the following options:

• Copy gcc's behavior, ignore that it's broken, and wait for someone to file a bug when a new version of clang breaks the Linux kernel.
• Come up with some way to reliably avoid situations like the testcase. Maybe some combination of error messages in situations where the compiler would generate broken code, generating inline implementations instead of libcalls, and avoiding certain optimizations. This is complicated, and I'm not sure it's really worth the effort when exactly one file in the whole world would use the option.
• Don't support -ffixed-x1, and convince the kernel maintainers not to require it.

[trong]

Emit a warning during call lowering if user requests to reserve an argument register and a function call is made.
And added test cases.

[trong]

• Copy gcc's behavior, ignore that it's broken, and wait for someone to file a bug when a new version of clang breaks the Linux kernel.

Current patch will emit a warning during call lowering if user requests to reserve an argument register and a function call is made. This should hopefully cover calls to builtins (like in your example). But as you pointed out there are probably other places where reserved registers can be spuriously used. I'm not opposed to continuously supporting these flags to build Linux kernel.

[efriedma]

lib/Target/AArch64/AArch64RegisterInfo.cpp

183 ↗

(On Diff #163949)

LLVM library code isn't allowed to write directly to stderr; you have to go through LLVMContext::diagnose.

If you're going to put user data into an error message, you have to escape it; we don't want to send terminal escape codes stderr.

Fix that, and make it an error, and I guess I'm fine with this, assuming it's enough to handle the kernel usage. It's still ugly, but given code that doesn't make any calls, argument registers aren't special, so it shouldn't miscompile.

[trong]

Function calls with argument registers reserved now emit errors instead of warnings.

[trong]

lib/Target/AArch64/AArch64RegisterInfo.cpp

183 ↗

(On Diff #163949)

As far as -ffixed-x# flags are concerned, this should be enough to support kernel usage.

[manojgupta]

lib/Target/AArch64/AArch64.td

102 ↗

(On Diff #163965)

Can you support reserving all of the registers instead of a subset?

[nickdesaulniers]

lib/Target/AArch64/AArch64RegisterInfo.cpp

173–177 ↗

(On Diff #163965)

std::any_of()

452–454 ↗

(On Diff #163965)

std::count()

lib/Target/AArch64/AArch64Subtarget.cpp

155 ↗

(On Diff #163965)

Sorry, where does 31 come from here?

[trong]

lib/Target/AArch64/AArch64.td

102 ↗

(On Diff #163965)

We probably can't reliably reserve some registers, e.g. x16, x17. And we would need more error handling for special usages of x19, x29 (maybe more). But I'd like to keep this change down to x1-7 since those are the ones that will actually be used.

[trong]

lib/Target/AArch64/AArch64RegisterInfo.cpp

452–454 ↗

(On Diff #163965)

Using BitVector::count() since we already have a BitVector.

lib/Target/AArch64/AArch64Subtarget.cpp

155 ↗

(On Diff #163965)

Changed this to use tablegen generated value.

[nickdesaulniers]

lib/Target/AArch64/AArch64RegisterInfo.cpp

173 ↗

(On Diff #164077)

consider using const iterators (cbegin, cend) if you're not modifying the iterated value.

lib/Target/AArch64/AArch64Subtarget.h

230 ↗

(On Diff #164077)

return type bool?

[trong]

lib/Target/AArch64/AArch64RegisterInfo.cpp

173 ↗

(On Diff #164077)

cbegin and cend are c++14 which isn't required to build llvm/clang

lib/Target/AArch64/AArch64Subtarget.h

230 ↗

(On Diff #164077)

oops

[nickdesaulniers]

It might be good to get one more signoff before merging. Maybe with someone with more experience than me.

[efriedma]

Needs a test showing we correctly print the error message for code with calls (with fastisel, globalisel, and sdag isel, for plain calls and runtime library calls like memcpy).

[kristof.beyls]

lib/Target/AArch64/AArch64.td

102 ↗

(On Diff #163965)

I've been told that there are more projects than just the linux kernel using -ffixed-reg command line options.
Indeed if I Google search with the following command, I see quite a few projects on github using this command line option (not necessarily for an AArch64 target):
"-ffixed-" site:github.com -"-ffixed-line-length" -"-ffixed-form"

That being said, I'm happy for this functionality to be added incrementally, for now just focussing on what happens to be needed for the linux kernel. As long as it's clear that at some point in the future this feature may need to be extended.

[trong]

Added test cases to make sure that correct errors are emitted with fastisel, globalisel, and sdag isel for plain calls and runtime library calls like memcpy.

lib/Target/AArch64/AArch64.td

102 ↗

(On Diff #163965)

Ack

[efriedma]

LGTM

[phosek]

lib/Target/AArch64/AArch64.td

102 ↗

(On Diff #163965)

We'd like to use this option as well for Fuchsia but we need the ability to reserve other registers beside x1-7 so as is this change is insufficient for us.

[efriedma]

lib/Target/AArch64/AArch64.td

102 ↗

(On Diff #163965)

If you have a need, feel free to post a followup patch. Each register needs to be evaluated separately to make sure reserving it actually works.

[trong]

Thank you all for your reviews! I don't have commit access. Could someone help me?

[nickdesaulniers]

Thanks for implementing this feature. I will commit it for you.

[nickdesaulniers]

Sorry, I had added you as author via:

$ git commit --amend --author="Tri Vo <[e-mail]>"
before
$ git svn dcommit

but it seems git-svn changed the authorship back to me. It seems it's a common convention to add:

Patch By: <author>

to the commit message to work around this limitation when committing on others behalf. I'm trying to see now if I can revert, then re-land with this note in the commit.

llvm/trunk/lib/Target/AArch64/AArch64.td

def FeatureZCZeroingFPWorkaround : SubtargetFeature<"zcz-fp-workaround",
     "HasZeroCycleZeroingFPWorkaround", "true",
     "The zero-cycle floating-point zeroing instruction has a bug">;
 
 def FeatureStrictAlign : SubtargetFeature<"strict-align",
                                           "StrictAlign", "true",
                                           "Disallow all unaligned memory "
                                           "access">;
 
-def FeatureReserveX18 : SubtargetFeature<"reserve-x18", "ReserveX18", "true",
-                                         "Reserve X18, making it unavailable "
-                                         "as a GPR">;
-
-def FeatureReserveX20 : SubtargetFeature<"reserve-x20", "ReserveX20", "true",
-                                         "Reserve X20, making it unavailable "
+foreach i = {1-7,18,20} in
+    def FeatureReserveX#i : SubtargetFeature<"reserve-x"#i, "ReserveXRegister["#i#"]", "true",
+                                             "Reserve X"#i#", making it unavailable "
                                              "as a GPR">;
 
 def FeatureUseAA : SubtargetFeature<"use-aa", "UseAA", "true",
                                     "Use alias analysis during codegen">;
 
 def FeatureBalanceFPOps : SubtargetFeature<"balance-fp-ops", "BalanceFPOps",
     "true",
     "balance mix of odd and even D-registers for fp multiply(-accumulate) ops">;
 

llvm/trunk/lib/Target/AArch64/AArch64CallLowering.cpp

bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
 
   // Create a temporarily-floating call instruction so we can add the implicit
   // uses of arg registers.
   auto MIB = MIRBuilder.buildInstrNoInsert(Callee.isReg() ? AArch64::BLR
                                                           : AArch64::BL);
   MIB.add(Callee);
 
   // Tell the call which registers are clobbered.
-  auto TRI = MF.getSubtarget().getRegisterInfo();
+  auto TRI = MF.getSubtarget<AArch64Subtarget>().getRegisterInfo();
   MIB.addRegMask(TRI->getCallPreservedMask(MF, F.getCallingConv()));
 
+  if (TRI->isAnyArgRegReserved(MF))
+    TRI->emitReservedArgRegCallError(MF);
+
   // Do the actual argument marshalling.
   SmallVector<unsigned, 8> PhysRegs;
   OutgoingArgHandler Handler(MIRBuilder, MRI, MIB, AssignFnFixed,
                              AssignFnVarArg);
   if (!handleAssignments(MIRBuilder, SplitArgs, Handler))
     return false;
 
   // Now we can add the actual call instruction to the correct basic block.

llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp

bool AArch64FastISel::fastLowerCall(CallLoweringInfo &CLI) {
   if (Callee && !computeCallAddress(Callee, Addr))
     return false;
 
   // Handle the arguments now that we've gotten them.
   unsigned NumBytes;
   if (!processCallArgs(CLI, OutVTs, NumBytes))
     return false;
 
+  const AArch64RegisterInfo *RegInfo = Subtarget->getRegisterInfo();
+  if (RegInfo->isAnyArgRegReserved(*MF))
+    RegInfo->emitReservedArgRegCallError(*MF);
+
   // Issue the call.
   MachineInstrBuilder MIB;
   if (Subtarget->useSmallAddressing()) {
     const MCInstrDesc &II = TII.get(Addr.getReg() ? AArch64::BLR : AArch64::BL);
     MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II);
     if (Symbol)
       MIB.addSym(Symbol, 0);
     else if (Addr.getGlobalValue())

llvm/trunk/lib/Target/AArch64/AArch64FrameLowering.cpp

if (i + 1 < Count) {
           (!RPI.IsGPR && AArch64::FPR64RegClass.contains(NextReg)))
         RPI.Reg2 = NextReg;
     }
 
     // If either of the registers to be saved is the lr register, it means that
     // we also need to save lr in the shadow call stack.
     if ((RPI.Reg1 == AArch64::LR || RPI.Reg2 == AArch64::LR) &&
         MF.getFunction().hasFnAttribute(Attribute::ShadowCallStack)) {
-      if (!MF.getSubtarget<AArch64Subtarget>().isX18Reserved())
+      if (!MF.getSubtarget<AArch64Subtarget>().isXRegisterReserved(18))
         report_fatal_error("Must reserve x18 to use shadow call stack");
       NeedShadowCallStackProlog = true;
     }
 
     // GPRs and FPRs are saved in pairs of 64-bit regs. We expect the CSI
     // list to come in sorted by frame index so that we can issue the store
     // pair instructions directly. Assert if we see anything otherwise.
     //

llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp

if (IsThisReturn) {
     Mask = TRI->getThisReturnPreservedMask(MF, CallConv);
     if (!Mask) {
       IsThisReturn = false;
       Mask = TRI->getCallPreservedMask(MF, CallConv);
     }
   } else
     Mask = TRI->getCallPreservedMask(MF, CallConv);
 
+  if (TRI->isAnyArgRegReserved(MF))
+    TRI->emitReservedArgRegCallError(MF);
+
   assert(Mask && "Missing call preserved mask for calling convention");
   Ops.push_back(DAG.getRegisterMask(Mask));
 
   if (InFlag.getNode())
     Ops.push_back(InFlag);
 
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
 

 }
 
 // FIXME? Maybe this could be a TableGen attribute on some registers and
 // this table could be generated automatically from RegInfo.
 unsigned AArch64TargetLowering::getRegisterByName(const char* RegName, EVT VT,
                                                   SelectionDAG &DAG) const {
   unsigned Reg = StringSwitch<unsigned>(RegName)
                        .Case("sp", AArch64::SP)
+                       .Case("x1", AArch64::X1)
+                       .Case("w1", AArch64::W1)
+                       .Case("x2", AArch64::X2)
+                       .Case("w2", AArch64::W2)
+                       .Case("x3", AArch64::X3)
+                       .Case("w3", AArch64::W3)
+                       .Case("x4", AArch64::X4)
+                       .Case("w4", AArch64::W4)
+                       .Case("x5", AArch64::X5)
+                       .Case("w5", AArch64::W5)
+                       .Case("x6", AArch64::X6)
+                       .Case("w6", AArch64::W6)
+                       .Case("x7", AArch64::X7)
+                       .Case("w7", AArch64::W7)
                        .Case("x18", AArch64::X18)
                        .Case("w18", AArch64::W18)
                        .Case("x20", AArch64::X20)
                        .Case("w20", AArch64::W20)
                        .Default(0);
-  if (((Reg == AArch64::X18 || Reg == AArch64::W18) &&
-      !Subtarget->isX18Reserved()) ||
+  if (((Reg == AArch64::X1 || Reg == AArch64::W1) &&
+      !Subtarget->isXRegisterReserved(1)) ||
+      ((Reg == AArch64::X2 || Reg == AArch64::W2) &&
+      !Subtarget->isXRegisterReserved(2)) ||
+      ((Reg == AArch64::X3 || Reg == AArch64::W3) &&
+      !Subtarget->isXRegisterReserved(3)) ||
+      ((Reg == AArch64::X4 || Reg == AArch64::W4) &&
+      !Subtarget->isXRegisterReserved(4)) ||
+      ((Reg == AArch64::X5 || Reg == AArch64::W5) &&
+      !Subtarget->isXRegisterReserved(5)) ||
+      ((Reg == AArch64::X6 || Reg == AArch64::W6) &&
+      !Subtarget->isXRegisterReserved(6)) ||
+      ((Reg == AArch64::X7 || Reg == AArch64::W7) &&
+      !Subtarget->isXRegisterReserved(7)) ||
+      ((Reg == AArch64::X18 || Reg == AArch64::W18) &&
+      !Subtarget->isXRegisterReserved(18)) ||
       ((Reg == AArch64::X20 || Reg == AArch64::W20) &&
-      !Subtarget->isX20Reserved()))
+      !Subtarget->isXRegisterReserved(20)))
     Reg = 0;
   if (Reg)
     return Reg;
   report_fatal_error(Twine("Invalid register name \""
                               + StringRef(RegName)  + "\"."));
 }
 
 SDValue AArch64TargetLowering::LowerRETURNADDR(SDValue Op,

llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h

 
 class AArch64RegisterInfo final : public AArch64GenRegisterInfo {
   const Triple &TT;
 
 public:
   AArch64RegisterInfo(const Triple &TT);
 
   bool isReservedReg(const MachineFunction &MF, unsigned Reg) const;
+  bool isAnyArgRegReserved(const MachineFunction &MF) const;
+  void emitReservedArgRegCallError(const MachineFunction &MF) const;
 
   /// Code Generation virtual methods...
   const MCPhysReg *getCalleeSavedRegs(const MachineFunction *MF) const override;
   const MCPhysReg *
   getCalleeSavedRegsViaCopy(const MachineFunction *MF) const;
   const uint32_t *getCallPreservedMask(const MachineFunction &MF,
                                        CallingConv::ID) const override;
 

llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp

 #include "MCTargetDesc/AArch64AddressingModes.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/RegisterScavenging.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/CodeGen/TargetFrameLowering.h"
 #include "llvm/Target/TargetOptions.h"
 
 using namespace llvm;
 
 #define GET_REGINFO_TARGET_DESC
 #include "AArch64GenRegisterInfo.inc"
AArch64RegisterInfo::getReservedRegs(const MachineFunction &MF) const {
   // FIXME: avoid re-calculating this every time.
   BitVector Reserved(getNumRegs());
   markSuperRegs(Reserved, AArch64::WSP);
   markSuperRegs(Reserved, AArch64::WZR);
 
   if (TFI->hasFP(MF) || TT.isOSDarwin())
     markSuperRegs(Reserved, AArch64::W29);
 
-  if (MF.getSubtarget<AArch64Subtarget>().isX18Reserved())
-    markSuperRegs(Reserved, AArch64::W18); // Platform register
-
-  if (MF.getSubtarget<AArch64Subtarget>().isX20Reserved())
-    markSuperRegs(Reserved, AArch64::W20); // Platform register
+  for (size_t i = 0; i < AArch64::GPR32commonRegClass.getNumRegs(); ++i) {
+    if (MF.getSubtarget<AArch64Subtarget>().isXRegisterReserved(i))
+      markSuperRegs(Reserved, AArch64::GPR32commonRegClass.getRegister(i));
+  }
 
   if (hasBasePointer(MF))
     markSuperRegs(Reserved, AArch64::W19);
 
   assert(checkAllSuperRegsMarked(Reserved));
   return Reserved;
 }
 
 bool AArch64RegisterInfo::isReservedReg(const MachineFunction &MF,
                                       unsigned Reg) const {
-  const AArch64FrameLowering *TFI = getFrameLowering(MF);
+  return getReservedRegs(MF)[Reg];
+}
 
-  switch (Reg) {
-  default:
-    break;
-  case AArch64::SP:
-  case AArch64::XZR:
-  case AArch64::WSP:
-  case AArch64::WZR:
-    return true;
-  case AArch64::X18:
-  case AArch64::W18:
-    return MF.getSubtarget<AArch64Subtarget>().isX18Reserved();
-  case AArch64::X19:
-  case AArch64::W19:
-    return hasBasePointer(MF);
-  case AArch64::X20:
-  case AArch64::W20:
-    return MF.getSubtarget<AArch64Subtarget>().isX20Reserved();
-  case AArch64::FP:
-  case AArch64::W29:
-    return TFI->hasFP(MF) || TT.isOSDarwin();
-  }
+bool AArch64RegisterInfo::isAnyArgRegReserved(const MachineFunction &MF) const {
+  // FIXME: Get the list of argument registers from TableGen.
+  static const MCPhysReg GPRArgRegs[] = { AArch64::X0, AArch64::X1, AArch64::X2,
+                                          AArch64::X3, AArch64::X4, AArch64::X5,
+                                          AArch64::X6, AArch64::X7 };
+  return std::any_of(std::begin(GPRArgRegs), std::end(GPRArgRegs),
+                     [this, &MF](MCPhysReg r){return isReservedReg(MF, r);});
 
-  return false;
+void AArch64RegisterInfo::emitReservedArgRegCallError(
+    const MachineFunction &MF) const {
+  const Function &F = MF.getFunction();
+  F.getContext().diagnose(DiagnosticInfoUnsupported{F, "AArch64 doesn't support"
+    " function calls if any of the argument registers is reserved."});
 }
 
 bool AArch64RegisterInfo::isAsmClobberable(const MachineFunction &MF,
                                           unsigned PhysReg) const {
   return !isReservedReg(MF, PhysReg);
 }
 
 bool AArch64RegisterInfo::isConstantPhysReg(unsigned PhysReg) const {
unsigned AArch64RegisterInfo::getRegPressureLimit(const TargetRegisterClass *RC,
   case AArch64::GPR32allRegClassID:
   case AArch64::GPR64spRegClassID:
   case AArch64::GPR64allRegClassID:
   case AArch64::GPR64RegClassID:
   case AArch64::GPR32commonRegClassID:
   case AArch64::GPR64commonRegClassID:
     return 32 - 1                                   // XZR/SP
               - (TFI->hasFP(MF) || TT.isOSDarwin()) // FP
-              - MF.getSubtarget<AArch64Subtarget>()
-                    .isX18Reserved() // X18 reserved as platform register
-              - MF.getSubtarget<AArch64Subtarget>()
-                    .isX20Reserved() // X20 reserved as platform register
+              - MF.getSubtarget<AArch64Subtarget>().getNumXRegisterReserved()
               - hasBasePointer(MF);  // X19
   case AArch64::FPR8RegClassID:
   case AArch64::FPR16RegClassID:
   case AArch64::FPR32RegClassID:
   case AArch64::FPR64RegClassID:
   case AArch64::FPR128RegClassID:
     return 32;
 

llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h

protected:
   uint16_t PrefetchDistance = 0;
   uint16_t MinPrefetchStride = 1;
   unsigned MaxPrefetchIterationsAhead = UINT_MAX;
   unsigned PrefFunctionAlignment = 0;
   unsigned PrefLoopAlignment = 0;
   unsigned MaxJumpTableSize = 0;
   unsigned WideningBaseCost = 0;
 
-  // ReserveX18 - X18 is not available as a general purpose register.
-  bool ReserveX18;
-
-  // ReserveX20 - X20 is not available as a general purpose register.
-  bool ReserveX20 = false;
+  // ReserveXRegister[i] - X#i is not available as a general purpose register.
+  BitVector ReserveXRegister;
 
   bool IsLittle;
 
   /// TargetTriple - What processor and OS we're targeting.
   Triple TargetTriple;
 
   AArch64FrameLowering FrameLowering;
   AArch64InstrInfo InstrInfo;
public:
   bool requiresStrictAlign() const { return StrictAlign; }
 
   bool isXRaySupported() const override { return true; }
 
   unsigned getMinVectorRegisterBitWidth() const {
     return MinVectorRegisterBitWidth;
   }
 
-  bool isX18Reserved() const { return ReserveX18; }
-  bool isX20Reserved() const { return ReserveX20; }
+  bool isXRegisterReserved(size_t i) const { return ReserveXRegister[i]; }
+  unsigned getNumXRegisterReserved() const { return ReserveXRegister.count(); }
   bool hasFPARMv8() const { return HasFPARMv8; }
   bool hasNEON() const { return HasNEON; }
   bool hasCrypto() const { return HasCrypto; }
   bool hasDotProd() const { return HasDotProd; }
   bool hasCRC() const { return HasCRC; }
   bool hasLSE() const { return HasLSE; }
   bool hasRAS() const { return HasRAS; }
   bool hasRDM() const { return HasRDM; }

llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp

void AArch64Subtarget::initializeProperties() {
   case Others: break;
   }
 }
 
 AArch64Subtarget::AArch64Subtarget(const Triple &TT, const std::string &CPU,
                                    const std::string &FS,
                                    const TargetMachine &TM, bool LittleEndian)
     : AArch64GenSubtargetInfo(TT, CPU, FS),
-      ReserveX18(AArch64::isX18ReservedByDefault(TT)), IsLittle(LittleEndian),
+      ReserveXRegister(AArch64::GPR64commonRegClass.getNumRegs()),
+      IsLittle(LittleEndian),
       TargetTriple(TT), FrameLowering(),
       InstrInfo(initializeSubtargetDependencies(FS, CPU)), TSInfo(),
       TLInfo(TM, *this) {
+  if (AArch64::isX18ReservedByDefault(TT))
+    ReserveXRegister.set(18);
+
   CallLoweringInfo.reset(new AArch64CallLowering(*getTargetLowering()));
   Legalizer.reset(new AArch64LegalizerInfo(*this));
 
   auto *RBI = new AArch64RegisterBankInfo(*getRegisterInfo());
 
   // FIXME: At this point, we can't rely on Subtarget having RBI.
   // It's awkward to mix passing RBI and the Subtarget; should we pass
   // TII/TRI as well?

llvm/trunk/test/CodeGen/AArch64/arm64-platform-reg.ll

 ; RUN: llc -mtriple=arm64-apple-ios -mattr=+reserve-x18 -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE --check-prefix=CHECK-RESERVE-X18
 ; RUN: llc -mtriple=arm64-freebsd-gnu -mattr=+reserve-x18 -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE --check-prefix=CHECK-RESERVE-X18
 ; RUN: llc -mtriple=aarch64-fuchsia -mattr=+reserve-x20 -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE --check-prefix=CHECK-RESERVE-X20
 ; RUN: llc -mtriple=aarch64-fuchsia -mattr=+reserve-x18,+reserve-x20 -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE --check-prefix=CHECK-RESERVE-X18 --check-prefix=CHECK-RESERVE-X20
 ; RUN: llc -mtriple=arm64-linux-gnu -o - %s | FileCheck %s
 ; RUN: llc -mtriple=aarch64-linux-android -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE --check-prefix=CHECK-RESERVE-X18
 ; RUN: llc -mtriple=aarch64-fuchsia -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE --check-prefix=CHECK-RESERVE-X18
 ; RUN: llc -mtriple=aarch64-windows -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE --check-prefix=CHECK-RESERVE-X18
 
+; Test reserve-x# options individually.
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x1 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X1
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x2 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X2
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x3 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X3
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x4 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X4
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x5 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X5
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x6 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X6
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x7 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X7
+
+; Test multiple of reserve-x# options together.
+; RUN: llc -mtriple=arm64-linux-gnu \
+; RUN: -mattr=+reserve-x1 \
+; RUN: -mattr=+reserve-x2 \
+; RUN: -mattr=+reserve-x18 \
+; RUN: -o - %s | FileCheck %s \
+; RUN: --check-prefix=CHECK-RESERVE \
+; RUN: --check-prefix=CHECK-RESERVE-X1 \
+; RUN: --check-prefix=CHECK-RESERVE-X2 \
+; RUN: --check-prefix=CHECK-RESERVE-X18
+
+; Test all reserve-x# options together.
+; RUN: llc -mtriple=arm64-linux-gnu \
+; RUN: -mattr=+reserve-x1 \
+; RUN: -mattr=+reserve-x2 \
+; RUN: -mattr=+reserve-x3 \
+; RUN: -mattr=+reserve-x4 \
+; RUN: -mattr=+reserve-x5 \
+; RUN: -mattr=+reserve-x6 \
+; RUN: -mattr=+reserve-x7 \
+; RUN: -mattr=+reserve-x18 \
+; RUN: -mattr=+reserve-x20 \
+; RUN: -o - %s | FileCheck %s \
+; RUN: --check-prefix=CHECK-RESERVE \
+; RUN: --check-prefix=CHECK-RESERVE-X1 \
+; RUN: --check-prefix=CHECK-RESERVE-X2 \
+; RUN: --check-prefix=CHECK-RESERVE-X3 \
+; RUN: --check-prefix=CHECK-RESERVE-X4 \
+; RUN: --check-prefix=CHECK-RESERVE-X5 \
+; RUN: --check-prefix=CHECK-RESERVE-X6 \
+; RUN: --check-prefix=CHECK-RESERVE-X7 \
+; RUN: --check-prefix=CHECK-RESERVE-X18 \
+; RUN: --check-prefix=CHECK-RESERVE-X20
+
 ; x18 is reserved as a platform register on Darwin but not on other
 ; systems. Create loads of register pressure and make sure this is respected.
 
 ; Also, fp must always refer to a valid frame record, even if it's not the one
 ; of the current function, so it shouldn't be used either.
 
 @var = global [30 x i64] zeroinitializer
 
 define void @keep_live() {
   %val = load volatile [30 x i64], [30 x i64]* @var
   store volatile [30 x i64] %val, [30 x i64]* @var
 
 ; CHECK: ldr x18
 ; CHECK: str x18
 
 ; CHECK-RESERVE-NOT: ldr fp
+; CHECK-RESERVE-X1-NOT: ldr x1,
+; CHECK-RESERVE-X2-NOT: ldr x2,
+; CHECK-RESERVE-X3-NOT: ldr x3,
+; CHECK-RESERVE-X4-NOT: ldr x4,
+; CHECK-RESERVE-X5-NOT: ldr x5,
+; CHECK-RESERVE-X6-NOT: ldr x6,
+; CHECK-RESERVE-X7-NOT: ldr x7,
 ; CHECK-RESERVE-X18-NOT: ldr x18
 ; CHECK-RESERVE-X20-NOT: ldr x20
 ; CHECK-RESERVE: Spill
 ; CHECK-RESERVE-NOT: ldr fp
+; CHECK-RESERVE-X1-NOT: ldr x1,
+; CHECK-RESERVE-X2-NOT: ldr x2,
+; CHECK-RESERVE-X3-NOT: ldr x3,
+; CHECK-RESERVE-X4-NOT: ldr x4,
+; CHECK-RESERVE-X5-NOT: ldr x5,
+; CHECK-RESERVE-X6-NOT: ldr x6,
+; CHECK-RESERVE-X7-NOT: ldr x7,
 ; CHECK-RESERVE-X18-NOT: ldr x18
 ; CHECK-RESERVE-X20-NOT: ldr x20
 ; CHECK-RESERVE: ret
   ret void
 }

llvm/trunk/test/CodeGen/AArch64/arm64-reserved-arg-reg-call-error.ll

+; RUN: not llc < %s -mtriple=arm64-linux-gnu -mattr=+reserve-x1 2>&1 | FileCheck %s
+; RUN: not llc < %s -mtriple=arm64-linux-gnu -mattr=+reserve-x1 -fast-isel 2>&1 | FileCheck %s
+; RUN: not llc < %s -mtriple=arm64-linux-gnu -mattr=+reserve-x1 -global-isel 2>&1 | FileCheck %s
+
+; CHECK: error:
+; CHECK-SAME: AArch64 doesn't support function calls if any of the argument registers is reserved.
+define void @call_function() {
+  call void @foo()
+  ret void
+}
+declare void @foo()
+
+; CHECK: error:
+; CHECK-SAME: AArch64 doesn't support function calls if any of the argument registers is reserved.
+define void @call_memcpy(i8* %out, i8* %in) {
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %out, i8* %in, i64 800, i1 false)
+  ret void
+}
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8*, i64, i1)