This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Support k-ext clang intrinsics
ClosedPublic

Authored by achieveartificialintelligence on Oct 28 2021, 6:05 PM.

Diff Detail

Event Timeline

achieveartificialintelligence requested review of this revision.Oct 28 2021, 6:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2021, 6:05 PM

The scalar crypto v1.0 builtins/intrinsics is still under discussion. Maybe we should wait for the final results?

craig.topper retitled this revision from Support k-ext clang intrinsics to [RISCV] Support k-ext clang intrinsics.Jan 27 2022, 12:54 AM
craig.topper added inline comments.Feb 3 2022, 9:16 AM
clang/include/clang/Basic/BuiltinsRISCV.def
74

Capital Z

clang/lib/Basic/Targets/RISCV.cpp
198 ↗(On Diff #405605)

This define doesn't seem very useful. It just tells you that you have some crypto instructions, but not which ones.

clang/lib/CodeGen/CGBuiltin.cpp
18900

Capital Z

achieveartificialintelligence marked 3 inline comments as done.

Address comments.

craig.topper added inline comments.Feb 13 2022, 7:25 PM
clang/include/clang/Basic/BuiltinsRISCV.def
80

Uc -> IUc

The I will make the frontend only accept constant arguments.

87

Ui -> IUi

91

Uc -> IUc

114

Uc -> IUc

achieveartificialintelligence marked 4 inline comments as done.

Address @craig.topper's comments.

Any intrinsic that takes an immediate that needs to be in certain range, needs to have code added to SemaChecking.cpp to validate the range. Look for the other places we call SemaBuiltinConstantArgRange in Sema::CheckRISCVBuiltinFunctionCall

clang/include/clang/Basic/BuiltinsRISCV.def
76

Nothing is preventing __builtin_riscv_zip/unzip from being used on RV64 which will crash. We don't have a "32bit" feature flag so we can't fix it from here. You'll need to go into clang/lib/Sema/SemaChecking.cpp and implement something like where X86 emits err_32_bit_builtin_64_bit_tgt. Same is true for any other 32-bit only intrinsic.

achieveartificialintelligence marked an inline comment as done.

Thanks for @craig.topper's detailed advice!

craig.topper added inline comments.Mar 2 2022, 2:20 PM
clang/include/clang/Basic/BuiltinsRISCV.def
75

Shouldn't brev8 be using "LiLi"?

76

Please add _32 to the end of zip and unzip.

clang/lib/Sema/SemaChecking.cpp
4092 ↗(On Diff #411676)

What about aes64ks1i's rnum?

clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkb.c
14

This should be long not int

clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb.c
2

Extra blank line at the top of the file

10

These should be i64

15

This should be long not int

Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 2:20 PM
achieveartificialintelligence marked 7 inline comments as done.

Address @craig.topper's comments. Thanks!

This revision is now accepted and ready to land.Mar 4 2022, 9:58 AM
This revision was landed with ongoing or failed builds.Mar 4 2022, 9:57 PM
This revision was automatically updated to reflect the committed changes.