[RISCV] Use OS-specific stack-guard ABI for Fuchsia

[mcgrathr]

Summary

Fuchsia provides a slot relative to tp for the stack-guard value,
which is cheaper to materialize than the default GOT load.

[phosek]

LGTM

llvm/lib/Target/RISCV/RISCVSubtarget.h

166

Nit: We usually don't repeat the name of the field in the documentation comment (AArch64Subtarget.h does that but it's not common across LLVM).

[jrtc27]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

14224

Just inline this, it has a single use. Negating the condition in getIRStackGuard will help avoid nesting, too.

llvm/lib/Target/RISCV/RISCVSubtarget.h

167

This now duplicates XLen... besides, don't we get this already from MCSubtargetInfo?

180

This seems unnecessary

llvm/test/CodeGen/RISCV/stack-protector-target.ll

2

Use update_llc_test_checks.py, and use ;; for non-lit/FileCheck comments

4

Only one prefix so don't use --check-prefix*es*, and if this is the only triple tested then you don't need a custom prefix anyway

Also the -o - is redundant when compiling stdin

6

C++ mangling needlessly obfuscates the test

7–9

Shorter

[mcgrathr]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

14224

It will be reused when the safe-stack ABI support is added for Fuchsia, and perhaps also later if other targets choose tp-offset ABIs for these things as they have on other machines.

llvm/lib/Target/RISCV/RISCVSubtarget.h

180

I'm following the model of other targets that have various is{Target,OS}* shorthands. They become helpful as the number of checks using them increases.

llvm/test/CodeGen/RISCV/stack-protector-target.ll

4

I've left the explicit prefix in anticipation of other target-specific checks being added here later, but made the other simplifications.

[mcgrathr]

cleanups per review

[mcgrathr]

rebase

[jrtc27]

llvm/test/CodeGen/RISCV/stack-protector-target.ll

12

nounwind will clean these up

29

I guess while you're fixing the above you can drop the nonnull, it doesn't do anything useful here

[jrtc27]

Latest feedback was not addressed when committing, and you gave hardly any time for anyone else who regularly works on or reviews changes for the RISC-V backend to comment

[mcgrathr]

In D143353#4105469, @jrtc27 wrote:

Latest feedback was not addressed when committing, and you gave hardly any time for anyone else who regularly works on or reviews changes for the RISC-V backend to comment

Sorry about that, I thought I'd done just as asked and hadn't seen the follow-up. https://reviews.llvm.org/D143360 has the recommended cleanups.

Thanks for the quick and thorough reviews. I'll wait a day for more comments on the cleanup change.

llvm/lib/Target/RISCV/RISCVISelLowering.h

public:
   bool isVScaleKnownToBeAPowerOfTwo() const override;
 
   bool isLegalScaleForGatherScatter(uint64_t Scale,
                                     uint64_t ElemSize) const override {
     // Scaled addressing not supported on indexed load/stores
     return Scale == 1;
   }
 
+  /// If the target has a standard location for the stack protector cookie,
+  /// returns the address of that location. Otherwise, returns nullptr.
+  Value *getIRStackGuard(IRBuilderBase &IRB) const override;
+
 private:
   /// RISCVCCAssignFn - This target-specific function extends the default
   /// CCValAssign with additional information used to lower RISC-V calling
   /// conventions.
   typedef bool RISCVCCAssignFn(const DataLayout &DL, RISCVABI::ABI,
                                unsigned ValNo, MVT ValVT, MVT LocVT,
                                CCValAssign::LocInfo LocInfo,
                                ISD::ArgFlagsTy ArgFlags, CCState &State,

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

 bool RISCVTargetLowering::preferScalarizeSplat(unsigned Opc) const {
   // Scalarize zero_ext and sign_ext might stop match to widening instruction in
   // some situation.
   if (Opc == ISD::ZERO_EXTEND || Opc == ISD::SIGN_EXTEND)
     return false;
   return true;
 }
 
+static Value *useTpOffset(IRBuilderBase &IRB, unsigned Offset) {
+  Module *M = IRB.GetInsertBlock()->getParent()->getParent();
+  Function *ThreadPointerFunc =
+      Intrinsic::getDeclaration(M, Intrinsic::thread_pointer);
+  return IRB.CreatePointerCast(
+      IRB.CreateConstGEP1_32(IRB.getInt8Ty(),
+                             IRB.CreateCall(ThreadPointerFunc), Offset),
+      IRB.getInt8PtrTy()->getPointerTo(0));
+}
+
+Value *RISCVTargetLowering::getIRStackGuard(IRBuilderBase &IRB) const {
+  // Fuchsia provides a fixed TLS slot for the stack cookie.
+  // <zircon/tls.h> defines ZX_TLS_STACK_GUARD_OFFSET with this value.
+  if (Subtarget.isTargetFuchsia())
+    return useTpOffset(IRB, -0x10);
+
+  return TargetLowering::getIRStackGuard(IRB);
+}
+
 #define GET_REGISTER_MATCHER
 #include "RISCVGenAsmMatcher.inc"
 
 Register
 RISCVTargetLowering::getRegisterByName(const char *RegName, LLT VT,
                                        const MachineFunction &MF) const {
   Register Reg = MatchRegisterAltName(RegName);
   if (Reg == RISCV::NoRegister)

llvm/lib/Target/RISCV/RISCVSubtarget.h

 
 protected:
   // GlobalISel related APIs.
   std::unique_ptr<CallLowering> CallLoweringInfo;
   std::unique_ptr<InstructionSelector> InstSelector;
   std::unique_ptr<LegalizerInfo> Legalizer;
   std::unique_ptr<RegisterBankInfo> RegBankInfo;
 
   // Return the known range for the bit length of RVV data registers as set
   // at the command line. A value of 0 means nothing is known about that particular
   // limit beyond what's implied by the architecture.
   // NOTE: Please use getRealMinVLen and getRealMaxVLen instead!
   unsigned getMaxRVVVectorSizeInBits() const;
   unsigned getMinRVVVectorSizeInBits() const;
 
 public:
   const CallLowering *getCallLowering() const override;
   InstructionSelector *getInstructionSelector() const override;
   const LegalizerInfo *getLegalizerInfo() const override;
   const RegisterBankInfo *getRegBankInfo() const override;
 
+  bool isTargetFuchsia() const { return getTargetTriple().isOSFuchsia(); }
+
   bool useConstantPoolForLargeInts() const;
 
   // Maximum cost used for building integers, integers will be put into constant
   // pool if exceeded.
   unsigned getMaxBuildIntsCost() const;
 
   unsigned getMaxLMULForFixedLengthVectors() const;
   bool useRVVForFixedLengthVectors() const;

llvm/test/CodeGen/RISCV/stack-protector-target.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+
+;; Test target-specific stack cookie location.
+;
+; RUN: llc -mtriple=riscv64-fuchsia < %s | FileCheck --check-prefix=FUCHSIA-RISCV64 %s
+
+define void @func() sspreq {
+; FUCHSIA-RISCV64-LABEL: func:
+; FUCHSIA-RISCV64:       # %bb.0:
+; FUCHSIA-RISCV64-NEXT:    addi sp, sp, -32
+; FUCHSIA-RISCV64-NEXT:    .cfi_def_cfa_offset 32
+; FUCHSIA-RISCV64-NEXT:    sd ra, 24(sp) # 8-byte Folded Spill
+; FUCHSIA-RISCV64-NEXT:    .cfi_offset ra, -8
+; FUCHSIA-RISCV64-NEXT:    ld a0, -16(tp)
+; FUCHSIA-RISCV64-NEXT:    sd a0, 16(sp)
+; FUCHSIA-RISCV64-NEXT:    addi a0, sp, 12
+; FUCHSIA-RISCV64-NEXT:    call capture@plt
+; FUCHSIA-RISCV64-NEXT:    ld a0, -16(tp)
+; FUCHSIA-RISCV64-NEXT:    ld a1, 16(sp)
+; FUCHSIA-RISCV64-NEXT:    bne a0, a1, .LBB0_2
+; FUCHSIA-RISCV64-NEXT:  # %bb.1: # %SP_return
+; FUCHSIA-RISCV64-NEXT:    ld ra, 24(sp) # 8-byte Folded Reload
+; FUCHSIA-RISCV64-NEXT:    addi sp, sp, 32
+; FUCHSIA-RISCV64-NEXT:    ret
+; FUCHSIA-RISCV64-NEXT:  .LBB0_2: # %CallStackCheckFailBlk
+; FUCHSIA-RISCV64-NEXT:    call __stack_chk_fail@plt
+  %1 = alloca i32, align 4
+  call void @capture(ptr nonnull %1)
+  ret void
+}
+
+declare void @capture(ptr)