[PATCH] D134050: [clang][RISCV][NFC][WIP/RFC] Move riscv-abi.cpp and riscv32-*abi.c tests to use update_cc_test_checks.py

Alex Bradbury via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 16 08:08:25 PDT 2022


asb created this revision.
asb added reviewers: reames, craig.topper, luismarques, kito-cheng.
Herald added subscribers: wingo, sunshaoce, pmatos, VincentWu, StephenFan, vkmr, frasercrmck, evandro, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.

I think our current set of ABI test files is organised about as well as it can be when relying on manual editing, but maintenance is error-prone and tedious. Having files containing tests that have the same signature for the ABIs indicated in the filename simplifies some things, but can create additional work when adding a new test that has behaviour that's worth testing across multiple ABI combinations. It's also not easy to tell at a glance whether a given function signature is tested across a sufficient set of ABIs, as you may need to look in multiple files to check for this.

This patch isn't ready for committing, and I don't propose committing it in its current form. As I've had to do some work on update_cc_test_checks.py to make it suitable for generating our ABI tests (see D133943 <https://reviews.llvm.org/D133943>) and will need to do further work to finalise this work, I wanted to get some feedback on the proposed direction.

This patch demonstrates the proposed direction I'd like to take these ABI tests in, with some caveats:

- Due to an update_cc_test_checks.py bug I haven't spent the time to track down and fix yet, I have to include at least one line from the function body for it to merge CHECK lines with the same content. I'd anticipate fixing this if we're agreed this is the right direction.
- riscv-abi.cpp is now generated using update_cc_test_checks.py, and also demonstrates the style of tests I'd like to move to - where all ABI variants are tested in a single file (potentially split by language features if it becomes too large) and we rely on update_cc_test_checks.py merging common CHECK lines for us.
- The riscv32-*abi.c tests are all regenerated using update_cc_test_checks.py. This represents the first step of my proposed changes - the next step would be to combine into one (or a small number of files based on language features / aspect of the ABI rather than target ABI variant) and use multiple RUN lines with overlapping check prefixes much like riscv-abi.cpp
  - It _probably_ makes sense to combine riscv32-*abi.c and riscv64*-abi.c tests - I've just made a start with riscv32* so as to get something for quick feedback

So - how do people feel about this direction? Potential options:

- Don't change existing tests and don't move to update_cc_test_checks
- Convert current tests to update_cc_test_checks but don't pursue any further refactoring/merging
- Convert current tests to update_cc_test_checks and then pursue something like I suggest above, combining more tests in the style of riscv-abi.cpp


https://reviews.llvm.org/D134050

Files:
  clang/test/CodeGen/RISCV/riscv-abi.cpp
  clang/test/CodeGen/RISCV/riscv32-ilp32-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32f-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32f-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv32-vararg.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D134050.460763.patch
Type: text/x-patch
Size: 99733 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220916/a5900280/attachment-0001.bin>


More information about the cfe-commits mailing list