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

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr marked 5 inline comments as done.
mcgrathr added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:14224
 
+static Value *useTpOffset(IRBuilderBase &IRB, unsigned Offset) {
+  Module *M = IRB.GetInsertBlock()->getParent()->getParent();
----------------
jrtc27 wrote:
> Just inline this, it has a single use. Negating the condition in getIRStackGuard will help avoid nesting, too.
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.



================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:183
+
+  bool isTargetFuchsia() const { return TargetTriple.isOSFuchsia(); }
 
----------------
jrtc27 wrote:
> This seems unnecessary
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.



================
Comment at: llvm/test/CodeGen/RISCV/stack-protector-target.ll:3
+;
+; RUN: llc -mtriple=riscv64-fuchsia < %s -o - | FileCheck --check-prefixes=FUCHSIA-RISCV64 %s
+
----------------
jrtc27 wrote:
> 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
I've left the explicit prefix in anticipation of other target-specific checks being added here later, but made the other simplifications.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143353/new/

https://reviews.llvm.org/D143353