[PATCH] D143353: [RISCV] Use OS-specific stack-guard ABI for Fuchsia
Jessica Clarke via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Feb 5 16:31:04 PST 2023
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
More information about the llvm-commits
mailing list