This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] [2/2] Add intrinsic for Zbb extension
Needs ReviewPublic

Authored by LevyHsu on Mar 25 2021, 12:20 AM.

Details

Summary

Implementation for RISC-V Zbb extension intrinsic.

Head files are included in the second patch in case the name needs to be changed.

RV32 / 64:
orc.b

__builtin_riscv_orc_b
__builtin_riscv32_orc_b
__builtin_riscv64_orc_b

_rv_orc_b
_rv32_orc_b
_rv64_orc_b

Diff Detail

Event Timeline

LevyHsu created this revision.Mar 25 2021, 12:20 AM
LevyHsu requested review of this revision.Mar 25 2021, 12:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 25 2021, 12:20 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
LevyHsu retitled this revision from [RISCV] [2/2] Add intrinsic for Zbr extension to [RISCV] [2/2] Add intrinsic for Zbb extension.Mar 25 2021, 12:21 AM
craig.topper added inline comments.Mar 25 2021, 12:30 AM
clang/lib/Headers/riscv_zbb_intrin.h
19

I think rather than int_xlen_t we want an int32_t version for RV32/RV64 and an int64_t version for RV64. I think it is safe to use the 64-bit orc.b instruction for int32_t on RV64 since each output byte is only affected by the bits in the same input byte. So we can put garbage in the upper 4 bytes and ignore the results in the upper bytes.

We couldn't do this for crc32(c) from Zbr.

clang/lib/Sema/SemaChecking.cpp
3398 ↗(On Diff #333220)

Variable names should start with capital letter and use CamelCase. No underscores.

3415 ↗(On Diff #333220)

Please run clang-format on this line. There are instructions here for running clang-format on a patch https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting

craig.topper added inline comments.Mar 25 2021, 12:41 AM
clang/lib/Headers/riscv_zbb_intrin.h
19

To do this you'll need to modify ReplaceNodeResults in RISCVISelLowering.cpp to detect the this intrinsic. I think there's already handling for some other intrinsics in there. Look for INTRINSIC_WO_CHAIN. For this you need to add an ANY_EXTEND to the operand, create a new INTRINSIC_WO_CHAIN node with i64 type, and truncate the result back to i32.

I think you'll need this in the RISCVTargetLowering constructor when Zbb is enabled. We only do it for the V extension right now.

setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::i32, Custom);
kito-cheng added inline comments.Mar 25 2021, 2:20 AM
clang/lib/Headers/riscv_zbb_intrin.h
19

In Claire's proposal, there is 3 different version of intrinsic for orc.b:

  • _rv_orc_b for int_xlen_t
  • _rv32_orc_b for int32_t only
  • _rv64_orc_b for int64_t only

https://github.com/riscv/riscv-bitmanip/blob/master/cproofs/rvintrin.h#L989

LevyHsu updated this revision to Diff 333801.Mar 29 2021, 3:16 AM
  1. Generated with git format-patch -o patches/ -2 HEAD -U999999
  2. clang/lib/Headers/riscv_zbb_intrin.h
    • Added corresponding 32/64 orcb intrinsic
    • include error message when user include this file directly.
LevyHsu edited the summary of this revision. (Show Details)Mar 29 2021, 3:17 AM
LevyHsu updated this revision to Diff 334073.Mar 30 2021, 1:41 AM
  1. clang/lib/Headers/riscv_zbb_intrin.h
    • Remove extra ports. now orc_b handles xlen op,
    • _rv64_orc_b and _rv32_orc_b for RV64
    • _rv32_orc_b for RV32
LevyHsu updated this revision to Diff 334328.Mar 30 2021, 7:43 PM
  1. clang/lib/Headers/riscv_zbb_intrin.h
    • Fixed llvm header and other style issues.
  2. clang/lib/Headers/rvintrin.h
    • Fixed llvm header and other style issues.