This is an archive of the discontinued LLVM Phabricator instance.

[NFC][RISCV] Add test showing wrong stack slot for GPR and RVV spilled registers
ClosedPublic

Authored by rogfer01 on Mar 17 2021, 11:37 AM.

Details

Summary

This testcase shows that we attempt to assign the same offset sp + 16 to two different stack objects.

The fix will come in a later change.

Diff Detail

Event Timeline

rogfer01 created this revision.Mar 17 2021, 11:37 AM
rogfer01 requested review of this revision.Mar 17 2021, 11:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2021, 11:37 AM
rogfer01 updated this revision to Diff 331545.Mar 18 2021, 6:36 AM

ChangeLog:

  • Simplify the MIR input
  • Split test in two for RV32 and RV64

I'm wondering if the commit message should indicate it's adding a test for the issue (though I realise space is at a premium). I was a bit confused at first.

llvm/test/CodeGen/RISCV/rvv/wrong-stack-slot-rv32.mir
21

nit: opening brace on a new line is a bit unusual?

rogfer01 updated this revision to Diff 332354.Mar 22 2021, 10:28 AM
rogfer01 retitled this revision from [NFC][RISCV] Wrong stack slot for GPR and RVV spilled registers to [NFC][RISCV] Add test showing wrong stack slot for GPR and RVV spilled registers.

ChangeLog:

  • Clarify the purpose of the change.
  • Adjust syntax of the IR input.
rogfer01 marked an inline comment as done.Mar 22 2021, 10:28 AM
frasercrmck accepted this revision.Mar 23 2021, 4:18 AM

LGTM though (sorry) maybe the tests could have a short comment explaining what they're testing (since they'll be fixed by a future patch anyway) and what to be wary of when the output changes.

This revision is now accepted and ready to land.Mar 23 2021, 4:18 AM
rogfer01 updated this revision to Diff 333906.Mar 29 2021, 9:29 AM

ChangeLog:

  • Describe a bit better the purpose of the test.

LGTM though (sorry) maybe the tests could have a short comment explaining what they're testing (since they'll be fixed by a future patch anyway) and what to be wary of when the output changes.

Sure, no problem. Thanks!

This revision was landed with ongoing or failed builds.Mar 29 2021, 10:03 AM
This revision was automatically updated to reflect the committed changes.