[CodeGen][RISCV] Change Shadow Call Stack Register to X3

[paulkirth]

Summary

ShadowCallStack implementation uses s2 register on RISC-V, but that
choice is problematic for reasons described in:

https://lists.riscv.org/g/sig-toolchains/message/544,
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/370, and
https://github.com/google/android-riscv64/issues/72

The concern over the register choice was also brought up in
https://reviews.llvm.org/D84414.

https://reviews.llvm.org/D84414#2228666 said:

"If the register choice is the only concern about this work, then I think
we can probably land it as-is and fixup the register choice if we see
major drawbacks later. Yes, it's an ABI issue, but on the other hand the
shadow call stack is not a standard ABI anyway.""

Since we have now found a sufficient reason to fixup the register
choice, we should go ahead and update the implementation. We propose
using x3(gp) which is now the platform register in the RISC-V ABI.

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

53–54

Can you add a FIXME here? Using x27 should hopefully remove this restriction

llvm/lib/TargetParser/RISCVTargetParser.cpp

104–105

X18->X27

[paulkirth]

Fix comment and add FIXME

[paulkirth]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

53–54

I've added the FIXME, but do we just want to remove this now? or is there some issues or chang under review I can ref in the FIXME?

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

53–54

We can remove it, but we should add a test for it.

[paulkirth]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

53–54

That makes sense. Thanks.

[paulkirth]

Remove outdated diagnostics and add tests for SCS + save/restore.

I'm not 100% on if this is the right approach for save/restore testing, so I'm hapy to back this bit out or add more cases.

[hiraditya]

Thanks for putting the patch.

[craig.topper]

llvm/test/CodeGen/RISCV/shadowcallstack.ll

184

Can we split this to a separate file? It doesn't look like it was generated by update_llc_test_checks.py and we don't want these checks on the test cases above.

[paulkirth]

llvm/test/CodeGen/RISCV/shadowcallstack.ll

184

Good point. This was taken directly from what looked to be the only relevant case in saverestore.ll. Tomorrow I will split this off into a standalone file. Do you know if there would be other relevant cases to consider from the save/restore tests?

[paulkirth]

Split out save/restore tests for SCS into its own file

[hiraditya]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

62

❤

[asb]

In principle I think this is good, but I see two considerations:

1. Sam was right that there's an ability to change the register after the fact, but 2.5 years have passed since then. We should make a good faith attempt to see if any downstream users are relying on the current ABI (e.g. the original contributors - @zzheng and @apazos).
2. Now that the issue has been raised in the psABI / toolchain-sig, it would be good if there were consensus on the "platform register" rather than changing this multiple times if some different register is decided.

I'd hope we can resolve this quickly enough that we don't need to consider e.g. adding a way to select the shadow call stack register, but if you _need_ this quickly to iterate on Android/Fuchsia then that might be the easiest route to land something very rapidly.

[hiraditya]

https://lists.riscv.org/g/sig-toolchains/message/548 has a proposal where X27 is one of the registers.

[MaskRay]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

53–54

Nit: Drop the period while updating the message. See https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

116

Nit: Drop the period while updating the message. See https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

llvm/test/CodeGen/RISCV/saverestore-scs.ll

1

;; appears a useful prefix to highlight non-RUN-non-CHECK comments. It also makes it clear Check is not a typo for CHECK: when we have some automation tools to detect stale CHECK lines.

[paulkirth]

Address comments.

• Remove . from warning message
• use ;; prefix in test

[paulkirth]

ugh. I accidentally commited some temporary work here.

[paulkirth]

Remove unrelated changes, due to bad commit.

[paulkirth]

Restore check lines in test.

[asb]

I've flagged this proposed change on Discourse (more the fact we're looking to change it, rather than the precise register we end up with).

[kito-cheng]

Sorry for cross post, but I guess some people might not follow closely in discourse (like me :P):

Another proposal from me is using gp as platform register: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/371

Some advantage on taking gp as platform register rather than other GPRs:

• Compiler doesn’t use gp register anywhere for now.
• All assembly files (which conform with current ABI) didn’t use that except the __global_pointer$ initialization code in CRT files.
• The main user is linker, linker will use that to perform linker relaxation, and we already have the command option to tune that off.

Potential issues:

• Loss the code size and performance gain from gp relaxation
  • The most gain from gp relaxation is embedded application, it’s different target audience as SCS, so this should not blocker issues.
  • Android is an example, it’s already disable GP relaxation at all, so we don’t have any loss for this case.
• Will it break any existing platform?
  • Treat gp as platform register is optional, it’s still default use as gp relaxation, so NO breakage on existing platform, but give the freedom of the platform to use gp register as other purpose if they don’t want gp relxation.
  • Added an attribute to let linker to help mixing up different gp usage objects, also linker could check that attribute to make sure gp relaxation is do-able or not.

[paulkirth]

update to use gp since it was chosen in psABI and sig-toolchain.

[craig.topper]

Does the kernel populate the initial value of the register for the shadow stack or is that done by a runtime inside the application?

[mcgrathr]

In D146463#4232214, @craig.topper wrote:

Does the kernel populate the initial value of the register for the shadow stack or is that done by a runtime inside the application?

This is a change to code generation and is orthogonal to how the runtime environment supports the ABI requirement. The details depend on the particular runtime environment, which is not the province of the compiler backend.

[craig.topper]

In D146463#4232221, @mcgrathr wrote:

In D146463#4232214, @craig.topper wrote:

Does the kernel populate the initial value of the register for the shadow stack or is that done by a runtime inside the application?

This is a change to code generation and is orthogonal to how the runtime environment supports the ABI requirement. The details depend on the particular runtime environment, which is not the province of the compiler backend.

I ask because GP is set in crtbegin.o on Linux today. So if its the kernel that populates it, then GP isn't valid choice for Linux without changing crtbegin.

[mcgrathr]

In D146463#4232252, @craig.topper wrote:

In D146463#4232221, @mcgrathr wrote:

In D146463#4232214, @craig.topper wrote:

Does the kernel populate the initial value of the register for the shadow stack or is that done by a runtime inside the application?

This is a change to code generation and is orthogonal to how the runtime environment supports the ABI requirement. The details depend on the particular runtime environment, which is not the province of the compiler backend.

I ask because GP is set in crtbegin.o on Linux today. So if its the kernel that populates it, then GP isn't valid choice for Linux without changing crtbegin.

To use ShadowCallStack, executables need to be built with -msmall-data-limit=0 and the runtime environment they use needs to meet the ABI. Neither of those things is provided out of the box for Linux, just as on AArch64 neither building with -ffixed-x18 nor a runtime environment meeting the ABI exists out of the box on Linux. If a runtime environment for Linux were to support the ShadowCallStack ABI, that would be done in startup code that runs after the code injected by crtbegin.o and before any code built for the ShadowCallStack is expected to be usable, such as in the libc startup code.

[paulkirth]

Remove blank lines added to test file.

[paulkirth]

@asb, @craig.topper, @jrtc27 Are there any remaining considerations for us here? From the discussions in psABI and sig-toolchain, I think we have a consensus that this is the approach we'll be taking for RISC-V. We'd prefer to correct this ASAP, so as to prevent future incompatibility/continuing to use a non-standard register.

[craig.topper]

LGTM other than that one comment.

llvm/lib/Target/RISCV/RISCVSubtarget.cpp

88

Drop the blank line here.

[mcgrathr]

I don't see changes here to make the Fuchsia & Android targets stop doing implicit -ffixed-x18 and to make them start doing implicit -msmall-data-limit=0 for non-PIC modes.

clang/lib/Driver/SanitizerArgs.cpp

549–550

For RISC-V this needs a similar check that the equivalent of -msmall-data-limit=0 is in force (that's the default under -fPIC but not other modes).

llvm/test/CodeGen/RISCV/reserved-regs.ll

60–63

gp is a fixed register for all RISC-V targets. What's important to check is that Fuchsia and Android disable the small-data-section behavior.

[paulkirth]

This comment has been deleted.

clang/lib/Driver/SanitizerArgs.cpp

549–550

I'll look into adding support for that.

llvm/lib/Target/RISCV/RISCVSubtarget.cpp

86

@mcgrathr This stops Fuchsia and Android from reserving X18. I'll look at doing something similar for -msmall-data-limit=0.

[jrtc27]

From an ABI/codegen perspective the small data limit doesn’t matter here, all it does is govern whether things go in .sdata or not. The linker is what chooses whether to use GP, which happens regardless of whether something is in .sdata, just things in .sdata are more likely to be picked, since GP is defined to be 0x800 from its start. What matters is that the linker is told not to do GP relaxation.

[asb]

In D146463#4247050, @paulkirth wrote:

@asb, @craig.topper, @jrtc27 Are there any remaining considerations for us here? From the discussions in psABI and sig-toolchain, I think we have a consensus that this is the approach we'll be taking for RISC-V. We'd prefer to correct this ASAP, so as to prevent future incompatibility/continuing to use a non-standard register.

It looks like we have strong consensus here. Could you update the patch to add a release note about this change please? (I guess it might merit inclusion in both the LLVM and the Clang release notes).

The RFC to check nobody downstream is concerned about changing the SCS register has been up for a week and a half with no concerns. You might consider leaving merging this until Monday to give it a full 2 week period, but I'm not opposed to going ahead before that. With the level of agreement we have it's hard to imagine changing direction and we want to change this default anyway - if there's a downstream user who needs to continue supporting the old SCS register then the logical path of action would be to add a new compiler option to enable that.

[asb]

Ok, looks like the consensus I thought we had isn't quite there on the psABI thread - Andrew Waterman has some concerns. We should let that discussion play out some more.

[paulkirth]

Fix formatting and update documentation.

[paulkirth]

Remove extra line.

[paulkirth]

Add default small-data-limit for Android and Fuchsia.

• update tests
• update clang driver

[paulkirth]

Remove unrelated whitespace changes.

[paulkirth]

@asb Are we happy with the state of consensus w.r.t. using x3? I think the lingering concerns from the psABI discussion have been resolved.

[asb]

In D146463#4255927, @paulkirth wrote:

@asb Are we happy with the state of consensus w.r.t. using x3? I think the lingering concerns from the psABI discussion have been resolved.

Yes, all LGTM now.

clang/lib/Driver/ToolChains/Clang.cpp

2062 ↗

(On Diff #511704)

Nit: Missing space after platforms

[mcgrathr]

lgtm with some nits and a few more test cases.

clang/docs/ShadowCallStack.rst

61 ↗

(On Diff #511704)

I think it would be worthwhile to say x3 (gp) here and perhaps everywhere that x3 is mentioned.
Since gp is always a reserved register from the compiler point of view, I think some different verbiage here about the RISC-V case and the interactions with -msmall-data-limit and linker behavior make sense.

clang/lib/Driver/SanitizerArgs.cpp

307

I'm guessing that "DSL" stands for "data small limit" or something, which is confusing since the thing is called small-data-limit, not data-small-limit. Anyway, this is the first use I've seen of DSL as an abbreviation related to any of this. I think it would be clear enough to just call it shadowCallStackConflicts since it's responsible for detecting conflicts between shadow-call-stack and whatever is relevant to that in the particular target.

314

This seems like it ought to parse the integer and then compare it to zero as an integer, rather than relying on one exact string representation of zero.

555

I'd just move all this per-target logic inside shadowCallStackConflicts and maybe just make it a void function called checkShadowCallStackConflcits or something.

clang/lib/Driver/ToolChains/Clang.cpp

2063 ↗

(On Diff #511704)

typo: "it's"

2065 ↗

(On Diff #511704)

I wonder if it wouldn't be more natural for the target hook to be defaultSmallDataLimit and return an integer where the base class returns 8 and the android/fuchsia subclasses return 0. It seems quite plausible that eventually different subtargets might have other tunings (for subtargets that do want to use gp relaxation).

clang/test/Driver/sanitizer-ld.c

746 ↗

(On Diff #511704)

I'd also add tests here for android and fuchsia targets with explicit -msmall-data-limit=<nonzero> being rejected by default but being allowed with -fno-sanitize=shadow-call-stack.

There should also be tests independent of shadow-call-stack that just verify at least for fuchsia targets that -msmall-data-limit=0 is passed by default for the default (PIE) case. Actually, it should also test that this remains so when -fno-sanitize=shadow-call-stack is passed, because it's still the standard target ABI that gp might be used for shadow-call-stack by interoperating code and so gp should never be used for something else without explicit -msmall-data-limit=... on fuchsia.

llvm/docs/ReleaseNotes.rst

147 ↗

(On Diff #511704)

Spaces before the parenthetical clauses.

llvm/include/llvm/TargetParser/RISCVTargetParser.h

44

Why not return an integer value instead of "is zero"?

[jrtc27]

clang/docs/ShadowCallStack.rst

157 ↗

(On Diff #511704)

This doesn't really achieve anything other than not do one optimisation (which may or not matter; the grouping of small data may have a noticeable cache locality effect)

clang/lib/Driver/SanitizerArgs.cpp

568

Why is this an error? It may be a misguided thing to enable but it is 100% supported to combine this. All the limit does is put things in .sdata, but they can still be addressed just fine.

clang/lib/Driver/ToolChains/Clang.cpp

2072 ↗

(On Diff #511704)

If you're modelling it on this warning (note, not an error) then this is crap too, there are legitimate reasons to use -G with -fPIC (and the -shared handling is even more questionable).

[paulkirth]

clang/lib/Driver/SanitizerArgs.cpp

307

Oops. Seems to be a typo. Thank you.

568

My understanding is that (whether they should or not) linkers are using the presence of the .sdata section to enable gp relaxation.

[jrtc27]

clang/lib/Driver/SanitizerArgs.cpp

568

That is not true

[jrtc27]

clang/lib/Driver/SanitizerArgs.cpp

568

Putting variables in .sdata merely makes it more likely that the linker will be able to relax accesses to them to use GP. Variables outside .sdata still can and will be relaxed if within range of whatever gets picked for __global_pointer$.

[paulkirth]

Remove -msmall-data-limit related changes. There was a misunderstanding on my part about default linker behavior.

clang/lib/Driver/SanitizerArgs.cpp

568

Yes, I seem to have had been under the wrong impression. I've dropped everything regarding -msmall-data-limit.

[paulkirth]

Remove dead declaration.

[paulkirth]

Add spaces to parantheticals. Shorten link to discussion on psABI.

clang/lib/Driver/SanitizerArgs.cpp

 bool SanitizerArgs::needsUnwindTables() const {
   return static_cast<bool>(Sanitizers.Mask & NeedsUnwindTables);
 }
 
 bool SanitizerArgs::needsLTO() const {
   return static_cast<bool>(Sanitizers.Mask & NeedsLTO);
 }
 
 SanitizerArgs::SanitizerArgs(const ToolChain &TC,
                              const llvm::opt::ArgList &Args,
                              bool DiagnoseErrors) {
   SanitizerMask AllRemove;      // During the loop below, the accumulated set of
                                 // sanitizers disabled by the current sanitizer
                                 // argument or any argument after it.
   SanitizerMask AllAddedKinds;  // Mask of all sanitizers ever enabled by
                                 // -fsanitize= flags (directly or via group
                                 // expansion), some of which may be disabled
                                 // later. Used to carefully prune
                                 // unused-argument diagnostics.
   SanitizerMask DiagnosedKinds; // All Kinds we have diagnosed up to now.
                                 // Used to deduplicate diagnostics.
   SanitizerMask Kinds;
   const SanitizerMask Supported = setGroupBits(TC.getSupportedSanitizers());
 
SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   if ((Kinds & NeedsLTO) && !D.isUsingLTO() && DiagnoseErrors) {
     D.Diag(diag::err_drv_argument_only_allowed_with)
         << lastArgumentForMask(D, Args, Kinds & NeedsLTO) << "-flto";
   }
 
   if ((Kinds & SanitizerKind::ShadowCallStack) &&
       ((TC.getTriple().isAArch64() &&
         !llvm::AArch64::isX18ReservedByDefault(TC.getTriple())) ||
        (TC.getTriple().isRISCV() &&
-        !llvm::RISCV::isX18ReservedByDefault(TC.getTriple()))) &&
-      !Args.hasArg(options::OPT_ffixed_x18) && DiagnoseErrors) {
+        !llvm::RISCV::isX27ReservedByDefault(TC.getTriple()))) &&
+      !Args.hasArg(options::OPT_ffixed_x27) && DiagnoseErrors) {
     D.Diag(diag::err_drv_argument_only_allowed_with)
         << lastArgumentForMask(D, Args, Kinds & SanitizerKind::ShadowCallStack)
-        << "-ffixed-x18";
+        << (TC.getTriple().isAArch64() ? "-ffixed-x18" : "-ffixed-x27");
   }
 
   // Report error if there are non-trapping sanitizers that require
   // c++abi-specific  parts of UBSan runtime, and they are not provided by the
   // toolchain. We don't have a good way to check the latter, so we just
   // check if the toolchan supports vptr.
   if (~Supported & SanitizerKind::Vptr) {
     SanitizerMask KindsToDiagnose = Kinds & ~TrappingKinds & NeedsUbsanCxxRt;
     // The runtime library supports the Microsoft C++ ABI, but only well enough
     // for CFI. FIXME: Remove this once we support vptr on Windows.
     if (TC.getTriple().isOSWindows())
       KindsToDiagnose &= ~SanitizerKind::CFI;
     if (KindsToDiagnose) {
       SanitizerSet S;
       S.Mask = KindsToDiagnose;
       if (DiagnoseErrors)
         D.Diag(diag::err_drv_unsupported_opt_for_target)
             << ("-fno-sanitize-trap=" + toString(S)) << TC.getTriple().str();
       Kinds &= ~KindsToDiagnose;
     }
   }
 

compiler-rt/test/shadowcallstack/lit.cfg.py

 config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.test']
 
 # Add clang substitutions.
 config.substitutions.append( ("%clang_noscs ", config.clang + ' -O0 -fno-sanitize=shadow-call-stack ' + config.target_cflags + ' ') )
 
 scs_arch_cflags = config.target_cflags
 if config.target_arch == 'aarch64':
   scs_arch_cflags += ' -ffixed-x18 '
+else if config.target_arch == 'riscv64':
+  scs_arch_cflags += ' -ffixed-x27 '
 config.substitutions.append( ("%clang_scs ", config.clang + ' -O0 -fsanitize=shadow-call-stack ' + scs_arch_cflags + ' ') )
 
-if config.host_os not in ['Linux'] or config.target_arch not in ['aarch64']:
+if config.host_os not in ['Linux'] or config.target_arch not in ['aarch64','riscv64']:
    config.unsupported = True

llvm/include/llvm/TargetParser/RISCVTargetParser.h

 bool checkTuneCPUKind(CPUKind Kind, bool IsRV64);
 CPUKind parseCPUKind(StringRef CPU);
 CPUKind parseTuneCPUKind(StringRef CPU, bool IsRV64);
 StringRef getMArchFromMcpu(StringRef CPU);
 void fillValidCPUArchList(SmallVectorImpl<StringRef> &Values, bool IsRV64);
 void fillValidTuneCPUArchList(SmallVectorImpl<StringRef> &Values, bool IsRV64);
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector<StringRef> &Features);
 
-bool isX18ReservedByDefault(const Triple &TT);
+bool isX27ReservedByDefault(const Triple &TT);
 
 } // namespace RISCV
 } // namespace llvm
 
 #endif

llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp

 }
 
 // To avoid the BP value clobbered by a function call, we need to choose a
 // callee saved register to save the value. RV32E only has X8 and X9 as callee
 // saved registers and X8 will be used as fp. So we choose X9 as bp.
 MCRegister getBPReg() { return RISCV::X9; }
 
 // Returns the register holding shadow call stack pointer.
-MCRegister getSCSPReg() { return RISCV::X18; }
+MCRegister getSCSPReg() { return RISCV::X27; }
 
 } // namespace RISCVABI
 
 namespace RISCVFeatures {
 
 void validate(const Triple &TT, const FeatureBitset &FeatureBits) {
   if (TT.isArch64Bit() && !FeatureBits[RISCV::Feature64Bit])
     report_fatal_error("RV64 target requires an RV64 CPU");

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/MC/MCDwarf.h"
 #include "llvm/Support/LEB128.h"
 
 #include <algorithm>
 
 using namespace llvm;
 
-// For now we use x18, a.k.a s2, as pointer to shadow call stack.
-// User should explicitly set -ffixed-x18 and not use x18 in their asm.
+// For now we use x27, a.k.a s11, as pointer to shadow call stack.
+// User should explicitly set -ffixed-x27 and not use x27 in their asm.
 static void emitSCSPrologue(MachineFunction &MF, MachineBasicBlock &MBB,
                             MachineBasicBlock::iterator MI,
                             const DebugLoc &DL) {
   if (!MF.getFunction().hasFnAttribute(Attribute::ShadowCallStack))
     return;
 
   const auto &STI = MF.getSubtarget<RISCVSubtarget>();
   const llvm::RISCVRegisterInfo *TRI = STI.getRegisterInfo();
   Register RAReg = TRI->getRARegister();
 
   // Do not save RA to the SCS if it's not saved to the regular stack,
   // i.e. RA is not at risk of being overwritten.
   std::vector<CalleeSavedInfo> &CSI = MF.getFrameInfo().getCalleeSavedInfo();
   if (llvm::none_of(
           CSI, [&](CalleeSavedInfo &CSR) { return CSR.getReg() == RAReg; }))
     return;
 
   Register SCSPReg = RISCVABI::getSCSPReg();
 
   auto &Ctx = MF.getFunction().getContext();
   if (!STI.isRegisterReservedByUser(SCSPReg)) {
     Ctx.diagnose(DiagnosticInfoUnsupported{
-        MF.getFunction(), "x18 not reserved by user for Shadow Call Stack."});
-    return;
-  }
-
-  const auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
-  if (RVFI->useSaveRestoreLibCalls(MF)) {
-    Ctx.diagnose(DiagnosticInfoUnsupported{
-        MF.getFunction(),
-        "Shadow Call Stack cannot be combined with Save/Restore LibCalls."});
+        MF.getFunction(), "x27 not reserved by user for Shadow Call Stack."});
     return;
   }
 
   const RISCVInstrInfo *TII = STI.getInstrInfo();
   bool IsRV64 = STI.hasFeature(RISCV::Feature64Bit);
   int64_t SlotSize = STI.getXLen() / 8;
   // Store return address to shadow call stack
-  // s[w|d]  ra, 0(s2)
-  // addi    s2, s2, [4|8]
+  // s[w|d]  ra, 0(s11)
+  // addi    s11, s11, [4|8]
   BuildMI(MBB, MI, DL, TII->get(IsRV64 ? RISCV::SD : RISCV::SW))
       .addReg(RAReg)
       .addReg(SCSPReg)
       .addImm(0)
       .setMIFlag(MachineInstr::FrameSetup);
   BuildMI(MBB, MI, DL, TII->get(RISCV::ADDI))
       .addReg(SCSPReg, RegState::Define)
       .addReg(SCSPReg)
       .addImm(SlotSize)
       .setMIFlag(MachineInstr::FrameSetup);
 
   // Emit a CFI instruction that causes SlotSize to be subtracted from the value
   // of the shadow stack pointer when unwinding past this frame.
   char DwarfSCSReg = TRI->getDwarfRegNum(SCSPReg, /*IsEH*/ true);
-  assert(DwarfSCSReg < 32 && "SCS Register should be < 32 (X18).");
+  assert(DwarfSCSReg < 32 && "SCS Register should be < 32 (X27).");
 
   char Offset = static_cast<char>(-SlotSize) & 0x7f;
   const char CFIInst[] = {
       dwarf::DW_CFA_val_expression,
       DwarfSCSReg, // register
       2,           // length
       static_cast<char>(unsigned(dwarf::DW_OP_breg0 + DwarfSCSReg)),
       Offset, // addend (sleb128)
if (llvm::none_of(
           CSI, [&](CalleeSavedInfo &CSR) { return CSR.getReg() == RAReg; }))
     return;
 
   Register SCSPReg = RISCVABI::getSCSPReg();
 
   auto &Ctx = MF.getFunction().getContext();
   if (!STI.isRegisterReservedByUser(SCSPReg)) {
     Ctx.diagnose(DiagnosticInfoUnsupported{
-        MF.getFunction(), "x18 not reserved by user for Shadow Call Stack."});
-    return;
-  }
-
-  const auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
-  if (RVFI->useSaveRestoreLibCalls(MF)) {
-    Ctx.diagnose(DiagnosticInfoUnsupported{
-        MF.getFunction(),
-        "Shadow Call Stack cannot be combined with Save/Restore LibCalls."});
+        MF.getFunction(), "x27 not reserved by user for Shadow Call Stack."});
     return;
   }
 
   const RISCVInstrInfo *TII = STI.getInstrInfo();
   bool IsRV64 = STI.hasFeature(RISCV::Feature64Bit);
   int64_t SlotSize = STI.getXLen() / 8;
   // Load return address from shadow call stack
-  // l[w|d]  ra, -[4|8](s2)
-  // addi    s2, s2, -[4|8]
+  // l[w|d]  ra, -[4|8](s11)
+  // addi    s11, s11, -[4|8]
   BuildMI(MBB, MI, DL, TII->get(IsRV64 ? RISCV::LD : RISCV::LW))
       .addReg(RAReg, RegState::Define)
       .addReg(SCSPReg)
       .addImm(-SlotSize)
       .setMIFlag(MachineInstr::FrameDestroy);
   BuildMI(MBB, MI, DL, TII->get(RISCV::ADDI))
       .addReg(SCSPReg, RegState::Define)
       .addReg(SCSPReg)

llvm/lib/Target/RISCV/RISCVSubtarget.cpp

RISCVSubtarget::RISCVSubtarget(const Triple &TT, StringRef CPU,
                                StringRef ABIName, unsigned RVVVectorBitsMin,
                                unsigned RVVVectorBitsMax,
                                const TargetMachine &TM)
     : RISCVGenSubtargetInfo(TT, CPU, TuneCPU, FS),
       RVVVectorBitsMin(RVVVectorBitsMin), RVVVectorBitsMax(RVVVectorBitsMax),
       FrameLowering(
           initializeSubtargetDependencies(TT, CPU, TuneCPU, FS, ABIName)),
       InstrInfo(*this), RegInfo(getHwMode()), TLInfo(TM, *this) {
-  if (RISCV::isX18ReservedByDefault(TT))
-    UserReservedRegister.set(RISCV::X18);
+  if (RISCV::isX27ReservedByDefault(TT))
+    UserReservedRegister.set(RISCV::X27);
 
   CallLoweringInfo.reset(new RISCVCallLowering(*getTargetLowering()));
   Legalizer.reset(new RISCVLegalizerInfo(*this));
 
   auto *RBI = new RISCVRegisterBankInfo(*getRegisterInfo());
   RegBankInfo.reset(RBI);
   InstSelector.reset(createRISCVInstructionSelector(
       *static_cast<const RISCVTargetMachine *>(&TM), *this, *RBI));
 }

llvm/lib/TargetParser/RISCVTargetParser.cpp

bool getCPUFeaturesExceptStdExt(CPUKind Kind,
   if (Info.is64Bit())
     Features.push_back("+64bit");
   else
     Features.push_back("-64bit");
 
   return true;
 }
 
-bool isX18ReservedByDefault(const Triple &TT) {
-  // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
+bool isX27ReservedByDefault(const Triple &TT) {
+  // X27 reserved for the ShadowCallStack ABI (even when not enabled).
   return TT.isOSFuchsia() || TT.isAndroid();
 }
 
 } // namespace RISCV
 } // namespace llvm

llvm/test/CodeGen/RISCV/reserved-regs.ll

 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x28 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X28
 ; RUN: llc -mtriple=riscv32 -mattr=+reserve-x29 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X29
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x29 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X29
 ; RUN: llc -mtriple=riscv32 -mattr=+reserve-x30 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X30
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x30 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X30
 ; RUN: llc -mtriple=riscv32 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 
-; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
-; RUN: llc -mtriple=riscv64-linux-android -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+;; Check that targets that reserve a register by default reserve the correct registers
+;; Android and Fuchsia reserve the ShadowCallStack register (S11/x27) by default.
+; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s -check-prefix=X27
+; RUN: llc -mtriple=riscv64-linux-android -verify-machineinstrs < %s | FileCheck %s -check-prefix=X27
 
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
 
 ; Used to exhaust all registers
 @var = global [32 x i64] zeroinitializer
 
 define void @foo() {

llvm/test/CodeGen/RISCV/saverestore-scs.ll

+; Check that shadow call stack doesn't interfere with save/restore
+
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x27 < %s | FileCheck %s -check-prefix=RV32I
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x27 < %s | FileCheck %s -check-prefix=RV64I
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x27 -mattr=+save-restore < %s | FileCheck %s -check-prefix=RV32I-SR
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x27 -mattr=+save-restore < %s | FileCheck %s -check-prefix=RV64I-SR
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x27 -mattr=+f,+save-restore -target-abi=ilp32f < %s | FileCheck %s -check-prefix=RV32I-FP-SR
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x27 -mattr=+f,+d,+save-restore -target-abi=lp64d < %s | FileCheck %s -check-prefix=RV64I-FP-SR
+
+@var2 = global [30 x i32] zeroinitializer
+
+define void @callee_scs() nounwind shadowcallstack {
+; RV32I-LABEL: callee_scs:
+; RV32I-NOT:     call t0, __riscv_save
+; RV32I-NOT:     tail __riscv_restore
+;
+; RV64I-LABEL: callee_scs:
+; RV64I-NOT:     call t0, __riscv_save
+; RV64I-NOT:     tail __riscv_restore
+;
+; RV32I-SR-LABEL: callee_scs:
+; RV32I-SR:         call t0, __riscv_save_11
+; RV32I-SR:         tail __riscv_restore_11
+;
+; RV64I-SR-LABEL: callee_scs:
+; RV64I-SR:         call t0, __riscv_save_11
+; RV64I-SR:         tail __riscv_restore_11
+;
+; RV32I-FP-SR-LABEL: callee_scs:
+; RV32I-FP-SR:         call t0, __riscv_save_11
+; RV32I-FP-SR:         tail __riscv_restore_11
+;
+; RV64I-FP-SR-LABEL: callee_scs:
+; RV64I-FP-SR:         call t0, __riscv_save_11
+; RV64I-FP-SR:         tail __riscv_restore_11
+  %val = load [30 x i32], ptr @var2
+  store volatile [30 x i32] %val, ptr @var2
+  ret void
+}
+

llvm/test/CodeGen/RISCV/shadowcallstack.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x27 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV32
-; RUN: llc -mtriple=riscv64 -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x27 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV64
 
 define void @f1() shadowcallstack {
 ; RV32-LABEL: f1:
 ; RV32:       # %bb.0:
 ; RV32-NEXT:    ret
 ;
 ; RV64-LABEL: f1:
; RV64-NEXT:    tail foo@plt
   ret void
 }
 
 declare i32 @bar()
 
 define i32 @f3() shadowcallstack {
 ; RV32-LABEL: f3:
 ; RV32:       # %bb.0:
-; RV32-NEXT:    sw ra, 0(s2)
-; RV32-NEXT:    addi s2, s2, 4
-; RV32-NEXT:    .cfi_escape 0x16, 0x12, 0x02, 0x82, 0x7c #
+; RV32-NEXT:    sw ra, 0(s11)
+; RV32-NEXT:    addi s11, s11, 4
+; RV32-NEXT:    .cfi_escape 0x16, 0x1b, 0x02, 0x8b, 0x7c #
 ; RV32-NEXT:    addi sp, sp, -16
 ; RV32-NEXT:    .cfi_def_cfa_offset 16
 ; RV32-NEXT:    sw ra, 12(sp) # 4-byte Folded Spill
 ; RV32-NEXT:    .cfi_offset ra, -4
 ; RV32-NEXT:    call bar@plt
 ; RV32-NEXT:    lw ra, 12(sp) # 4-byte Folded Reload
 ; RV32-NEXT:    addi sp, sp, 16
-; RV32-NEXT:    lw ra, -4(s2)
-; RV32-NEXT:    addi s2, s2, -4
-; RV32-NEXT:    .cfi_restore s2
+; RV32-NEXT:    lw ra, -4(s11)
+; RV32-NEXT:    addi s11, s11, -4
+; RV32-NEXT:    .cfi_restore s11
 ; RV32-NEXT:    ret
 ;
 ; RV64-LABEL: f3:
 ; RV64:       # %bb.0:
-; RV64-NEXT:    sd ra, 0(s2)
-; RV64-NEXT:    addi s2, s2, 8
-; RV64-NEXT:    .cfi_escape 0x16, 0x12, 0x02, 0x82, 0x78 #
+; RV64-NEXT:    sd ra, 0(s11)
+; RV64-NEXT:    addi s11, s11, 8
+; RV64-NEXT:    .cfi_escape 0x16, 0x1b, 0x02, 0x8b, 0x78 #
 ; RV64-NEXT:    addi sp, sp, -16
 ; RV64-NEXT:    .cfi_def_cfa_offset 16
 ; RV64-NEXT:    sd ra, 8(sp) # 8-byte Folded Spill
 ; RV64-NEXT:    .cfi_offset ra, -8
 ; RV64-NEXT:    call bar@plt
 ; RV64-NEXT:    ld ra, 8(sp) # 8-byte Folded Reload
 ; RV64-NEXT:    addi sp, sp, 16
-; RV64-NEXT:    ld ra, -8(s2)
-; RV64-NEXT:    addi s2, s2, -8
-; RV64-NEXT:    .cfi_restore s2
+; RV64-NEXT:    ld ra, -8(s11)
+; RV64-NEXT:    addi s11, s11, -8
+; RV64-NEXT:    .cfi_restore s11
 ; RV64-NEXT:    ret
   %res = call i32 @bar()
   %res1 = add i32 %res, 1
   ret i32 %res
 }
 
 define i32 @f4() shadowcallstack {
 ; RV32-LABEL: f4:
 ; RV32:       # %bb.0:
-; RV32-NEXT:    sw ra, 0(s2)
-; RV32-NEXT:    addi s2, s2, 4
-; RV32-NEXT:    .cfi_escape 0x16, 0x12, 0x02, 0x82, 0x7c #
+; RV32-NEXT:    sw ra, 0(s11)
+; RV32-NEXT:    addi s11, s11, 4
+; RV32-NEXT:    .cfi_escape 0x16, 0x1b, 0x02, 0x8b, 0x7c #
 ; RV32-NEXT:    addi sp, sp, -16
 ; RV32-NEXT:    .cfi_def_cfa_offset 16
 ; RV32-NEXT:    sw ra, 12(sp) # 4-byte Folded Spill
 ; RV32-NEXT:    sw s0, 8(sp) # 4-byte Folded Spill
 ; RV32-NEXT:    sw s1, 4(sp) # 4-byte Folded Spill
-; RV32-NEXT:    sw s3, 0(sp) # 4-byte Folded Spill
+; RV32-NEXT:    sw s2, 0(sp) # 4-byte Folded Spill
 ; RV32-NEXT:    .cfi_offset ra, -4
 ; RV32-NEXT:    .cfi_offset s0, -8
 ; RV32-NEXT:    .cfi_offset s1, -12
-; RV32-NEXT:    .cfi_offset s3, -16
+; RV32-NEXT:    .cfi_offset s2, -16
 ; RV32-NEXT:    call bar@plt
 ; RV32-NEXT:    mv s0, a0
 ; RV32-NEXT:    call bar@plt
 ; RV32-NEXT:    mv s1, a0
 ; RV32-NEXT:    call bar@plt
-; RV32-NEXT:    mv s3, a0
+; RV32-NEXT:    mv s2, a0
 ; RV32-NEXT:    call bar@plt
 ; RV32-NEXT:    add s0, s0, s1
-; RV32-NEXT:    add a0, s3, a0
+; RV32-NEXT:    add a0, s2, a0
 ; RV32-NEXT:    add a0, s0, a0
 ; RV32-NEXT:    lw ra, 12(sp) # 4-byte Folded Reload
 ; RV32-NEXT:    lw s0, 8(sp) # 4-byte Folded Reload
 ; RV32-NEXT:    lw s1, 4(sp) # 4-byte Folded Reload
-; RV32-NEXT:    lw s3, 0(sp) # 4-byte Folded Reload
+; RV32-NEXT:    lw s2, 0(sp) # 4-byte Folded Reload
 ; RV32-NEXT:    addi sp, sp, 16
-; RV32-NEXT:    lw ra, -4(s2)
-; RV32-NEXT:    addi s2, s2, -4
-; RV32-NEXT:    .cfi_restore s2
+; RV32-NEXT:    lw ra, -4(s11)
+; RV32-NEXT:    addi s11, s11, -4
+; RV32-NEXT:    .cfi_restore s11
 ; RV32-NEXT:    ret
 ;
 ; RV64-LABEL: f4:
 ; RV64:       # %bb.0:
-; RV64-NEXT:    sd ra, 0(s2)
-; RV64-NEXT:    addi s2, s2, 8
-; RV64-NEXT:    .cfi_escape 0x16, 0x12, 0x02, 0x82, 0x78 #
+; RV64-NEXT:    sd ra, 0(s11)
+; RV64-NEXT:    addi s11, s11, 8
+; RV64-NEXT:    .cfi_escape 0x16, 0x1b, 0x02, 0x8b, 0x78 #
 ; RV64-NEXT:    addi sp, sp, -32
 ; RV64-NEXT:    .cfi_def_cfa_offset 32
 ; RV64-NEXT:    sd ra, 24(sp) # 8-byte Folded Spill
 ; RV64-NEXT:    sd s0, 16(sp) # 8-byte Folded Spill
 ; RV64-NEXT:    sd s1, 8(sp) # 8-byte Folded Spill
-; RV64-NEXT:    sd s3, 0(sp) # 8-byte Folded Spill
+; RV64-NEXT:    sd s2, 0(sp) # 8-byte Folded Spill
 ; RV64-NEXT:    .cfi_offset ra, -8
 ; RV64-NEXT:    .cfi_offset s0, -16
 ; RV64-NEXT:    .cfi_offset s1, -24
-; RV64-NEXT:    .cfi_offset s3, -32
+; RV64-NEXT:    .cfi_offset s2, -32
 ; RV64-NEXT:    call bar@plt
 ; RV64-NEXT:    mv s0, a0
 ; RV64-NEXT:    call bar@plt
 ; RV64-NEXT:    mv s1, a0
 ; RV64-NEXT:    call bar@plt
-; RV64-NEXT:    mv s3, a0
+; RV64-NEXT:    mv s2, a0
 ; RV64-NEXT:    call bar@plt
 ; RV64-NEXT:    add s0, s0, s1
-; RV64-NEXT:    add a0, s3, a0
+; RV64-NEXT:    add a0, s2, a0
 ; RV64-NEXT:    addw a0, s0, a0
 ; RV64-NEXT:    ld ra, 24(sp) # 8-byte Folded Reload
 ; RV64-NEXT:    ld s0, 16(sp) # 8-byte Folded Reload
 ; RV64-NEXT:    ld s1, 8(sp) # 8-byte Folded Reload
-; RV64-NEXT:    ld s3, 0(sp) # 8-byte Folded Reload
+; RV64-NEXT:    ld s2, 0(sp) # 8-byte Folded Reload
 ; RV64-NEXT:    addi sp, sp, 32
-; RV64-NEXT:    ld ra, -8(s2)
-; RV64-NEXT:    addi s2, s2, -8
-; RV64-NEXT:    .cfi_restore s2
+; RV64-NEXT:    ld ra, -8(s11)
+; RV64-NEXT:    addi s11, s11, -8
+; RV64-NEXT:    .cfi_restore s11
 ; RV64-NEXT:    ret
   %res1 = call i32 @bar()
   %res2 = call i32 @bar()
   %res3 = call i32 @bar()
   %res4 = call i32 @bar()
   %res12 = add i32 %res1, %res2
   %res34 = add i32 %res3, %res4
   %res1234 = add i32 %res12, %res34
   ret i32 %res1234
 }
 
 define i32 @f5() shadowcallstack nounwind {
 ; RV32-LABEL: f5:
 ; RV32:       # %bb.0:
-; RV32-NEXT:    sw ra, 0(s2)
-; RV32-NEXT:    addi s2, s2, 4
+; RV32-NEXT:    sw ra, 0(s11)
+; RV32-NEXT:    addi s11, s11, 4
 ; RV32-NEXT:    addi sp, sp, -16
 ; RV32-NEXT:    sw ra, 12(sp) # 4-byte Folded Spill
 ; RV32-NEXT:    call bar@plt
 ; RV32-NEXT:    lw ra, 12(sp) # 4-byte Folded Reload
 ; RV32-NEXT:    addi sp, sp, 16
-; RV32-NEXT:    lw ra, -4(s2)
-; RV32-NEXT:    addi s2, s2, -4
+; RV32-NEXT:    lw ra, -4(s11)
+; RV32-NEXT:    addi s11, s11, -4
 ; RV32-NEXT:    ret
 ;
 ; RV64-LABEL: f5:
 ; RV64:       # %bb.0:
-; RV64-NEXT:    sd ra, 0(s2)
-; RV64-NEXT:    addi s2, s2, 8
+; RV64-NEXT:    sd ra, 0(s11)
+; RV64-NEXT:    addi s11, s11, 8
 ; RV64-NEXT:    addi sp, sp, -16
 ; RV64-NEXT:    sd ra, 8(sp) # 8-byte Folded Spill
 ; RV64-NEXT:    call bar@plt
 ; RV64-NEXT:    ld ra, 8(sp) # 8-byte Folded Reload
 ; RV64-NEXT:    addi sp, sp, 16
-; RV64-NEXT:    ld ra, -8(s2)
-; RV64-NEXT:    addi s2, s2, -8
+; RV64-NEXT:    ld ra, -8(s11)
+; RV64-NEXT:    addi s11, s11, -8
 ; RV64-NEXT:    ret
   %res = call i32 @bar()
   %res1 = add i32 %res, 1
   ret i32 %res
 }
+
+