[PATCH] D69721: [RISCV] Add CFI-related tests

[Sam Elliott via Phabricator via llvm-commits]

lenary added a comment.

I think the two TODOs will get lost if they're in the test files, they should go near the top of a .cpp file where the missing functions are implemented.

Otherwise, two minor nits.



================
Comment at: llvm/test/CodeGen/RISCV/frame-info.ll:5
+; RUN: llc -mtriple=riscv32 -frame-pointer=all -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32-WITHFP %s
 
----------------
Can you add a rv64 test with frame pointers too? Given the assertions are auto-generated, more coverage is better.


================
Comment at: llvm/test/CodeGen/RISCV/frame-info.ll:125
+
+; TODO: fix use of .cfi_restore with wrong CFAs
+
----------------
This TODO should not go into the tests, it should go into the implementation.


================
Comment at: llvm/test/CodeGen/RISCV/frame-info.ll:212
 
-declare void @bar(i8*)
+declare void @foo()
+declare void @bar()
----------------
Please can we give these functions more descriptive names?


================
Comment at: llvm/test/CodeGen/RISCV/vararg.ll:1766
+
+; TODO: also use the split-sp update approach for varargs
+
----------------
This TODO should go into `RISCVFrameLowering.cpp`, not the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69721