[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: | FileCheck %s --check-prefix=RV32
----------------
As I said before, please just use `-mtriple=riscv32`. The `-unknown-elf` is implied, irrelevant and wastes space, so all the OS-independent CodeGen tests just specify the CPU.


================
Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:3
+; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: | FileCheck %s --check-prefix=RV32
+
----------------
Two extra spaces to indent the | is the predominant style.


================
Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:4
+; RUN: | FileCheck %s --check-prefix=RV32
+
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+reserve-x18 -verify-machineinstrs < %s \
----------------
Delete this blank line.


================
Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:12
+; RV32-NEXT:    ret
+; RV32-NOT:     x18
+;
----------------
pcc wrote:
> Shouldn't it be looking for `s2` since that's how `x18` is spelled in assembly?
The -NOTs shouldn't even exist, this isn't how you use `update_llc_test_checks.py`. But yes, by default that's how it'll be printed unless you disable printing aliases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414