This is an archive of the discontinued LLVM Phabricator instance.

[1/3][Clang][RISCV] Add `__riscv_` prefix for vread, vwrite, vlenb, vsetvl, and vsetvlmax
ClosedPublic

Authored by eopXD on Jan 19 2023, 12:11 AM.

Details

Summary

This commit adds prefix for intrinsics that are defined through
HeaderCode under riscv_vector.td.

This is the 1st commit of a patch-set to add __riscv_ for all RVV
intrinsics.

This follows the naming guideline under riscv-c-api-doc to add the
__riscv_ suffix for all RVV intrinsics.

Pull Request:
riscv-non-isa/riscv-c-api-doc#31
riscv-non-isa/rvv-intrinsic-doc#189

Depends on D142016.

Diff Detail

Event Timeline

eopXD created this revision.Jan 19 2023, 12:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2023, 12:11 AM
eopXD requested review of this revision.Jan 19 2023, 12:11 AM
eopXD edited the summary of this revision. (Show Details)Jan 19 2023, 12:55 AM
eopXD updated this revision to Diff 490416.Jan 19 2023, 1:32 AM

Update test cases under test/CodeGen/RISCV/rvv-intrinsics-handcrafted

eopXD updated this revision to Diff 490417.Jan 19 2023, 1:39 AM

Bump CI.

Could you split testcase update to a separated patch?

craig.topper added inline comments.Jan 19 2023, 10:52 PM
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaadd.c
5–14 ↗(On Diff #490417)

Can we use 2 underscores instead of 3?

craig.topper added inline comments.Jan 19 2023, 10:56 PM
clang/include/clang/Basic/riscv_vector.td
1535

Not related to this patch, but this enum should probably be using double underscores for its names.

(For some reason Phab won't let me comment inline from the Changeset View page, just does nothing when I click reply or click a line...)

Or just leave the test names alone? The __riscv_ is for namespacing the intrinsics, you don't need to namespace the tests when they're already in a RISC-V test directory.

Not related to this patch, but this enum should probably be using double underscores for its names.

Thanks for mentioning this, probably should start a PR under rvv-intrinsic-doc before having a patch here.

@craig.topper @kito-cheng I don't have a preference here, is any of __riscv_RVV_CSR/ __riscv_rvv_csr_type / __riscv_rvv_csr_t suitable?

I don't have a preference here, is any of riscv_RVV_CSR/ riscv_rvv_csr_type / __riscv_rvv_csr_t suitable?

Either is fine to me at this stage, I expect they will be drop soon.

Agree with @jrtc27 about the test function name, we could keep the function name in the test file, that's also reduce lots of diff which is not really necessary.

e.g.
vint8mf8_t test_vaadd_vv_i8mf8(vint8mf8_t op1, vint8mf8_t op2, size_t vl) keep same name rather than rename to vint8mf8_t test___riscv_vaadd_vv_i8mf8(vint8mf8_t op1, vint8mf8_t op2, size_t vl)

eopXD retitled this revision from [Clang][RISCV] Add `__riscv_` prefix for all RVV intrinsics to [WIP][1/N][Clang][RISCV] Add `__riscv_` prefix for vread, vwrite, vsetvl, and vsetvlmax.Jan 26 2023, 8:34 AM
eopXD edited the summary of this revision. (Show Details)
eopXD updated this revision to Diff 492464.Jan 26 2023, 8:35 AM

Update code. Split the single patch into a patch-set of smaller steps.

eopXD added a comment.Jan 26 2023, 8:36 AM

As requested, @jrtc27 @kito-cheng, the patch does not alter the function name now.

eopXD retitled this revision from [WIP][1/N][Clang][RISCV] Add `__riscv_` prefix for vread, vwrite, vsetvl, and vsetvlmax to [WIP][1/N][Clang][RISCV] Add `__riscv_` prefix for vread, vwrite, vlenb, vsetvl, and vsetvlmax.Jan 26 2023, 8:58 AM
eopXD updated this revision to Diff 492474.Jan 26 2023, 9:01 AM

Update test case for vlenb.

eopXD retitled this revision from [WIP][1/N][Clang][RISCV] Add `__riscv_` prefix for vread, vwrite, vlenb, vsetvl, and vsetvlmax to [1/3][Clang][RISCV] Add `__riscv_` prefix for vread, vwrite, vlenb, vsetvl, and vsetvlmax.Jan 27 2023, 1:48 AM
This revision is now accepted and ready to land.Jan 29 2023, 10:43 PM
This revision was landed with ongoing or failed builds.Jan 31 2023, 1:06 AM
This revision was automatically updated to reflect the committed changes.