This is an archive of the discontinued LLVM Phabricator instance.

[Clang][RISCV][NFC] Reorganize test case for rvv intrinsics
ClosedPublic

Authored by eopXD on Jan 7 2023, 7:21 AM.

Details

Summary

The file hierarchy is reorganized into:

├── rvv-intrinsics-autogenerated
│   ├── non-policy
│   │   ├── non-overloaded
│   │   └── overloaded
│   └── policy
│       ├── non-overloaded
│       └── overloaded
└── rvv-intrinsics-handcrafted

Separating auto-generated test cases and hand-craft ones. The
auto-generated ones are basic API tests generated from the intrinsic
generator [0].

This re-organization allows direct copy-and-paste from the produced
outputs of the generator in future API changes, which is discussed
and needs to be implemented towards a v1.0 RVV intrinsic.

[0] https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/181

Diff Detail

Event Timeline

eopXD created this revision.Jan 7 2023, 7:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 7 2023, 7:21 AM
eopXD requested review of this revision.Jan 7 2023, 7:21 AM
eopXD edited the summary of this revision. (Show Details)Jan 7 2023, 7:29 AM
eopXD retitled this revision from [NFC][Clang][RISCV] Reorganize test case for rvv intrinsics to [Clang][RISCV][NFC] Reorganize test case for rvv intrinsics.Jan 7 2023, 7:31 AM
eopXD updated this revision to Diff 487094.Jan 7 2023, 7:32 AM

Rebase to latest main.

eopXD added a comment.Jan 7 2023, 7:38 AM

The testing time difference is shown below. I personally think 3 more minutes in testing is tolerable when considering check-all. The testing time increase comes from full test coverage to the intrinsics. Before this patch, some test cases only includes parts of the {SEW, LMUL} combination of an intrinsic to reduce avoid long compile time. We no longer suffer from long compile time as the intrinsics are now lazily added.

Before

$ bin/llvm-lit ../clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded ../clang/test/CodeGen/RISCV/rvv-intrinsics ../clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted
llvm-lit: /scratch/eopc/upstream-llvm-project/llvm/utils/lit/lit/llvm/config.py:459: note: using clang: /scratch/eopc/upstream-llvm-project/build/bin/clang
-- Testing: 326 tests, 48 workers --
Testing Time: 25.83s
  Passed: 326

After

$ bin/llvm-lit ../clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated
llvm-lit: /scratch/eopc/upstream-llvm-project/llvm/utils/lit/lit/llvm/config.py:459: note: using clang: /scratch/eopc/upstream-llvm-project/build/bin/clang
-- Testing: 566 tests, 48 workers --
Testing Time: 225.61s
  Passed: 566
eopXD edited the summary of this revision. (Show Details)Jan 7 2023, 7:47 AM
eopXD edited the summary of this revision. (Show Details)
eopXD added a comment.Jan 7 2023, 7:51 AM

The patch application failure probably is due to the large amount of changes in this patch.

eopXD added a comment.Jan 7 2023, 8:05 AM

I have created a branch under my forked repo. for people to checkout this commit conveniently.

https://github.com/eopXD/llvm-project/tree/reorganize-testcase

craig.topper added inline comments.Jan 8 2023, 1:46 PM
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaadd.c
3

Why do we need to zfh and zvfh on integer tests?

eopXD updated this revision to Diff 487317.Jan 9 2023, 12:58 AM
eopXD edited the summary of this revision. (Show Details)

Update test cases. Don't add target feature zfh and zvfh for intrinsics that does not have a floating-point type variant.

eopXD marked an inline comment as done.Jan 9 2023, 1:00 AM

Please split vmulh* from vmul.c. Remove the vmul tests from the handcrafted vmul.c and vmul-eew64.c. Rename those to vmulh.c and vmulh-eew64.c

eopXD updated this revision to Diff 487683.Jan 9 2023, 10:21 PM

Address comment from Craig.

Also separate test case according to their mnemonics.

eopXD updated this revision to Diff 488055.Jan 10 2023, 6:23 PM

Rebase to latest main.

eopXD updated this revision to Diff 488061.Jan 10 2023, 6:54 PM

Recover mis-deleted handcrafted test case for vmulh, vmulhu, and vmulhsu on e8 ~ e64 with zve32x.

craig.topper accepted this revision.Jan 10 2023, 7:10 PM

LGTM as far as I'm able to review in phabricator

This revision is now accepted and ready to land.Jan 10 2023, 7:10 PM
This revision was landed with ongoing or failed builds.Jan 10 2023, 11:14 PM
This revision was automatically updated to reflect the committed changes.