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

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 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();
----------------
Just inline this, it has a single use. Negating the condition in getIRStackGuard will help avoid nesting, too.


================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:167
+  /// TargetTriple - What processor and OS we're targeting.
+  Triple TargetTriple;
+
----------------
This now duplicates XLen... besides, don't we get this already from MCSubtargetInfo?


================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:183
+
+  bool isTargetFuchsia() const { return TargetTriple.isOSFuchsia(); }
 
----------------
This seems unnecessary


================
Comment at: llvm/test/CodeGen/RISCV/stack-protector-target.ll:1
+; Test target-specific stack cookie location.
+;
----------------
Use update_llc_test_checks.py, and use ;; for non-lit/FileCheck comments


================
Comment at: llvm/test/CodeGen/RISCV/stack-protector-target.ll:3
+;
+; RUN: llc -mtriple=riscv64-fuchsia < %s -o - | FileCheck --check-prefixes=FUCHSIA-RISCV64 %s
+
----------------
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


================
Comment at: llvm/test/CodeGen/RISCV/stack-protector-target.ll:5
+
+define void @_Z1fv() sspreq {
+entry:
----------------
C++ mangling needlessly obfuscates the test


================
Comment at: llvm/test/CodeGen/RISCV/stack-protector-target.ll:6-8
+entry:
+  %x = alloca i32, align 4
+  call void @_Z7CapturePi(ptr nonnull %x)
----------------
Shorter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143353