This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add CFI-related tests
ClosedPublic

Authored by luismarques on Nov 1 2019, 10:17 AM.

Details

Summary

Adds tests necessary to properly show the impact of other patches that affect the emission of CFI directives.

Diff Detail

Event Timeline

luismarques created this revision.Nov 1 2019, 10:17 AM
lenary added a comment.Nov 4 2019, 6:51 AM

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.

llvm/test/CodeGen/RISCV/frame-info.ll
5

Can you add a rv64 test with frame pointers too? Given the assertions are auto-generated, more coverage is better.

125

This TODO should not go into the tests, it should go into the implementation.

212

Please can we give these functions more descriptive names?

llvm/test/CodeGen/RISCV/vararg.ll
1766

This TODO should go into RISCVFrameLowering.cpp, not the tests.

Address review concerns

luismarques marked 5 inline comments as done.Nov 4 2019, 11:13 AM
luismarques added inline comments.
llvm/test/CodeGen/RISCV/vararg.ll
1766

This was actually not a split-sp issue, but a more general issue of constant materialization of stack addresses. Updated the comment. Given the broader scope, I'm letting it remain simply as a test comment.

lenary accepted this revision.Nov 7 2019, 5:33 AM

LGTM, Thanks!

Don't forget to add [NFC] into the commit message.

This revision is now accepted and ready to land.Nov 7 2019, 5:33 AM
This revision was automatically updated to reflect the committed changes.
luismarques marked an inline comment as done.