This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Reduce the number of RISCV vector builtins by an order of magnitude.
ClosedPublic

Authored by craig.topper on Oct 19 2021, 3:42 PM.

Details

Summary

All but 2 of the vector builtins are only used by clang_builtin_alias.
When using clang_builtin_alias, the type string of the builtin is never
checked. Only the types in the function definition used for the alias
are checked.

This patch takes advantage of this to share a single builtin for
many different types. We already used type overloads on the IR intrinsic
so the codegen for the builtins that are being merge were already
the same. This extends the type overloading to the builtins.

I had to make a few tweaks to make this work.
-Floating point vector-vector vmerge now uses the vmerge intrinsic
instead of the vfmerge intrinsic. New isel patterns and tests are
added to support this.
-The SemaChecking for the immediate of vset_v/vget_v has been removed.
Determining the valid range is harder now. I've added masking to
ManualCodegen to ensure valid IR for invalid input.

This reduces the number of builtins from ~25000 to ~1100.

Diff Detail

Event Timeline

craig.topper created this revision.Oct 19 2021, 3:42 PM
craig.topper requested review of this revision.Oct 19 2021, 3:42 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 19 2021, 3:42 PM
craig.topper edited the summary of this revision. (Show Details)Oct 19 2021, 3:43 PM
HsiangKai added inline comments.Oct 20 2021, 1:02 AM
llvm/test/CodeGen/RISCV/rvv/vmerge-rv32.ll
1358

vmerge.vvm is for integer vectors, doesn't it? Why does it work for floating point vector types?

frasercrmck added inline comments.Oct 20 2021, 2:11 AM
llvm/test/CodeGen/RISCV/rvv/vmerge-rv32.ll
1358

It's just a vselect in llvm terms, so doesn't care what its (vector) inputs are. The only reason we need vfmerge is when one operand is a scalar floating-point register.

Minor typo in the description: differnet

Does this help with compile times, binary sizes, etc?

khchen added inline comments.Oct 20 2021, 3:14 AM
clang/include/clang/Basic/riscv_vector.td
120

nit: we need to update this comment.

HsiangKai added inline comments.Oct 20 2021, 4:08 AM
llvm/test/CodeGen/RISCV/rvv/vmerge-rv32.ll
1358

Got it. I think I am misled by the title "Vector Integer Merge Instructions" in the spec. Thanks.

Minor typo in the description: differnet

Does this help with compile times, binary sizes, etc?

After this patch, we may be able to consider removing all type ending C APIs. Otherwise, I don't think it is helpful with compile times or binary sizes, etc.

craig.topper added a comment.EditedOct 20 2021, 8:58 AM

Minor typo in the description: differnet

Does this help with compile times, binary sizes, etc?

This reduces the clang binary size by 2.34 megabytes according to my local release+asserts build.

I think there might be a small decrease in lit test time. If not, I don't see any reason it would increase with this patch. The header file size does decrease.

craig.topper edited the summary of this revision. (Show Details)Oct 20 2021, 8:59 AM

Minor typo in the description: differnet

Does this help with compile times, binary sizes, etc?

This reduces the clang binary size by 2.34 megabytes according to my local release+asserts build.

I think there might be a small decrease in lit test time. If not, I don't see any reason it would increase with this patch. The header file size does decrease.

My fault. This patch decreases the binary size. The testing time increases due to D112020. This patch decreases the header file size indeed.

It looks good to me. Wait for others' opinions.

Looks good to me too. Thanks a lot @craig.topper !

HsiangKai accepted this revision.Oct 24 2021, 7:23 PM
This revision is now accepted and ready to land.Oct 24 2021, 7:23 PM