[RISCV] [1/2] Add intrinsic for Zbb extension

[LevyHsu]

Summary

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

RV32 / 64:
orc.b

__builtin_riscv_orc_b_32
__builtin_riscv_orc_b_64

_rv_orc_b
_rv32_orc_b
_rv64_orc_b

[craig.topper]

Please upload the patch using arcanist or using -U999999 when generating the diff.

[LevyHsu]

1. Generated with git format-patch -o patches/ -2 HEAD -U999999
2. clang/lib/Sema/SemaChecking.cpp
   • Rewrote CheckRISCVBuiltinFunctionCall
3. clang/lib/CodeGen/CGBuiltin.cpp
   • IntrinsicTypes = {ResultType};
4. llvm/include/llvm/IR/IntrinsicsRISCV.td
   • The second llvm_any_ty has been changed to LLVMMatchType<0>
5. llvm/lib/Target/RISCV/RISCVISelLowering.cpp
   • ANY_EXTEND both op0 and op 0 then truncate back to i32 when orc32b under RV64
6. llvm/test/CodeGen/RISCV/rv32zbb-intrinsic.ll llvm/test/CodeGen/RISCV/rv64zbb-intrinsic.ll clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbb.c clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
   • Added corresponding testcases.

[craig.topper]

clang/include/clang/Basic/BuiltinsRISCV.def

21

Can we select between the 32 and 64 bit version in the header based on __riscv_xlen so we only need 2 builtins, rather than 3?

22

All RISCV builtins should start with "__builtin_riscv_". I don't think we should use riscv32/riscv64 here. Especially since the 32-bit version is available on RV64. So I think these should be named

builtin_riscv_orc_b_32 and builtin_riscv_orc_b_64.

23

Ideally this would be "experimental-zbb,64bit" but I'm not sure that the frontend knows about "64bit".

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

4175 ↗

(On Diff #333798)

"LHS" stands for "left hand side", but operand 0 is the intrinsic ID which should already be i64. Only operand 1 needs to be extended. So just do

SDValue NewOp1 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, N->getOperand(1));
SDValue Res = DAG.getNode(N->getOpcode(), DL, MVT::i64, N->getOperand(0), NewOp1);

[LevyHsu]

1. llvm/lib/Target/RISCV/RISCVISelLowering.cpp
   • Fixed mishandling on op0
2. clang/include/clang/Basic/BuiltinsRISCV.def clang/lib/CodeGen/CGBuiltin.cpp
   • Reduce port to 2 versions for 32/64 only.
3. clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbb.c clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
   • Remove extra tests

[craig.topper]

clang/lib/Sema/SemaChecking.cpp

3418 ↗

(On Diff #334072)

LLVM coding style does not allow _ in variable names.

3427 ↗

(On Diff #334072)

Drop the else. Since the if has a continue, control flow won't reach here if the 'if' is true.

clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c

26

Why _2?

[LevyHsu]

1. clang/lib/Sema/SemaChecking.cpp
   • Fixed var name & loop
2. clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
   • renamed function

[craig.topper]

I think this looks good to me. Anyone else have any comments?

[Jim]

llvm/include/llvm/IR/IntrinsicsRISCV.td

26

How about put it between Atomics and Vector to follow canonical order?

[asb]

Can I just check the reasoning on the naming? I see that the bitmanip 0.93 spec proposes _{rv,rv32,rv64}_{opname} intrinsics. Does the __builtin__{riscv,riscv32,riscv64}_opname format match what GCC are doing / planning to do here? Precedent for RVV, for other archs, or something else?

Just trying to determine to what these names are an open item of discussion vs matching something else.

EDIT: I see Craig commented on this point in https://reviews.llvm.org/D99009#2660456

[craig.topper]

In D99320#2661285, @asb wrote:

Can I just check the reasoning on the naming? I see that the bitmanip 0.93 spec proposes _{rv,rv32,rv64}_{opname} intrinsics. Does the __builtin__{riscv,riscv32,riscv64}_opname format match what GCC are doing / planning to do here? Precedent for RVV, for other archs, or something else?

Just trying to determine to what these names are an open item of discussion vs matching something else.

EDIT: I see Craig commented on this point in https://reviews.llvm.org/D99009#2660456

Yeah the big open for naming is the name of the intrinsic header. Whether it should be rvintrin.h or riscv_intrinsic.h.

The builtins in this patch are

builtin_riscv_orc_b_32
builtin_riscv_orc_b_64

I've update the description summary to reflect this since it change during development.

The Zbr patch uses the following without a 32/64 suffix.
__builtin_riscv_crc32_b/w/d/q

There's a difference because orc.b needs to be available for 32-bit on RV32 and RV64 per the spec so we have two builtins. crc32.b intrinsics in the spec are defined only for xlen so we have a single builtin.

@kito-cheng do you know how gcc will name builtins?

[kito-cheng]

Created an issue for continue discuses on riscv-c-api-doc
https://github.com/riscv/riscv-c-api-doc/issues/19

[craig.topper]

I'm going to approve this. If we need to change the builtin names in the future that's easy enough to do.

clang/include/clang/Basic/BuiltinsRISCV.def

 
 #include "clang/Basic/riscv_vector_builtins.inc"
 
+// Zbb extension
+TARGET_BUILTIN(__builtin_riscv_orc_b, "LiLi", "nc", "experimental-zbb")
+
 #undef BUILTIN
 #undef TARGET_BUILTIN

clang/include/clang/Basic/DiagnosticSemaKinds.td

   "calling %0 is a violation of trusted computing base '%1'">,
   InGroup<DiagGroup<"tcb-enforcement">>;
 
-// RISC-V V-extension
-def err_riscvv_builtin_requires_v : Error<
-   "builtin requires 'V' extension support to be enabled">;
+// RISC-V builtin required extension warning
+def err_riscv_builtin_requires_extension : Error<
+  "builtin requires %0 extension support to be enabled">;
 } // end of sema component.

clang/lib/CodeGen/CGBuiltin.cpp

   llvm::SmallVector<llvm::Type *, 2> IntrinsicTypes;
   switch (BuiltinID) {
 #include "clang/Basic/riscv_vector_builtin_cg.inc"
+
+  // Zbb
+  case RISCV::BI__builtin_riscv_orc_b:
+    ID = Intrinsic::riscv_orc_b;
+    IntrinsicTypes = {ResultType, Ops[0]->getType()};
+    break;
+  default: {
+    llvm_unreachable("unexpected builtin ID");
+    return nullptr;
+  } // default  
   }
 
   assert(ID != Intrinsic::not_intrinsic);

clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbb.c

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple riscv32 -target-feature +experimental-zbb -emit-llvm %s -o - \
+// RUN:     | FileCheck %s  -check-prefix=RV32ZBB
+
+// RV32ZBB-LABEL: @orcb(
+// RV32ZBB-NEXT:  entry:
+// RV32ZBB-NEXT:    [[A_ADDR:%.*]] = alloca i32, align 4
+// RV32ZBB-NEXT:    store i32 [[A:%.*]], i32* [[A_ADDR]], align 4
+// RV32ZBB-NEXT:    [[TMP0:%.*]] = load i32, i32* [[A_ADDR]], align 4
+// RV32ZBB-NEXT:    [[TMP1:%.*]] = call i32 @llvm.riscv.orc.b.i32.i32(i32 [[TMP0]])
+// RV32ZBB-NEXT:    ret i32 [[TMP1]]
+//
+long orcb(long a) {
+  return __builtin_riscv_orc_b(a);
+}

clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-zbb -emit-llvm %s -o - \
+// RUN:     | FileCheck %s  -check-prefix=RV64ZBB
+
+// RV64ZBB-LABEL: @orcb(
+// RV64ZBB-NEXT:  entry:
+// RV64ZBB-NEXT:    [[A_ADDR:%.*]] = alloca i64, align 8
+// RV64ZBB-NEXT:    store i64 [[A:%.*]], i64* [[A_ADDR]], align 8
+// RV64ZBB-NEXT:    [[TMP0:%.*]] = load i64, i64* [[A_ADDR]], align 8
+// RV64ZBB-NEXT:    [[TMP1:%.*]] = call i64 @llvm.riscv.orc.b.i64.i64(i64 [[TMP0]])
+// RV64ZBB-NEXT:    ret i64 [[TMP1]]
+//
+long orcb(long a) {
+  return __builtin_riscv_orc_b(a);
+}

llvm/include/llvm/IR/IntrinsicsRISCV.td

 //
 //===----------------------------------------------------------------------===//
 
+//===----------------------------------------------------------------------===//
+// RISC-V Bitmanip (Bit Manipulation) Extension
+// Zbb extension part
+
+let TargetPrefix = "riscv" in {
+  
+    class BitMan_GPR_Intrinsics
+        : Intrinsic<[llvm_any_ty], [llvm_any_ty],
+                    [IntrNoMem, IntrSpeculatable, IntrWillReturn]>;
+  
+    def int_riscv_orc_b : BitMan_GPR_Intrinsics;
+
+} // TargetPrefix = "riscv"
+
 //===----------------------------------------------------------------------===//
 // Atomics
 

llvm/lib/Target/RISCV/RISCVInstrInfo.td

 
 /// Generic pattern classes
 
+class PatGpr<SDPatternOperator OpNode, RVInst Inst>
+    : Pat<(OpNode GPR:$rs1), (Inst GPR:$rs1)>;
 class PatGprGpr<SDPatternOperator OpNode, RVInst Inst>
     : Pat<(OpNode GPR:$rs1, GPR:$rs2), (Inst GPR:$rs1, GPR:$rs2)>;
 class PatGprSimm12<SDPatternOperator OpNode, RVInstI Inst>

llvm/lib/Target/RISCV/RISCVInstrInfoB.td

                    (SRLIWPat GPR:$rs1, (i64 16)))),
           (PACKUW GPR:$rs1, GPR:$rs2)>;
 } // Predicates = [HasStdExtZbp, IsRV64]
+
+let Predicates = [HasStdExtZbb] in {
+def : PatGpr<int_riscv_orc_b, ORCB>;
+} // Predicates = [HasStdExtZbb]

llvm/test/CodeGen/RISCV/rv32zbb-intrinsic.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-b -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32IB
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zbb -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32IBB
+
+declare i32 @llvm.riscv.orc.b.i32(i32)
+
+define i32 @orcb(i32 %a) nounwind {
+; RV32IB-LABEL: orcb:
+; RV32IB:       # %bb.0:
+; RV32IB-NEXT:    orc.b a0, a0
+; RV32IB-NEXT:    ret
+;
+; RV32IBB-LABEL: orcb:
+; RV32IBB:       # %bb.0:
+; RV32IBB-NEXT:    orc.b a0, a0
+; RV32IBB-NEXT:    ret
+  %tmp = call i32 @llvm.riscv.orc.b.i32(i32 %a)
+ ret i32 %tmp
+}

llvm/test/CodeGen/RISCV/rv64zbb-intrinsic.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-b -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64IB
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zbb -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64IBB
+
+declare i64 @llvm.riscv.orc.b.i64(i64)
+
+define i64 @orcb(i64 %a) nounwind {
+; RV64IB-LABEL: orcb:
+; RV64IB:       # %bb.0:
+; RV64IB-NEXT:    orc.b a0, a0
+; RV64IB-NEXT:    ret
+;
+; RV64IBB-LABEL: orcb:
+; RV64IBB:       # %bb.0:
+; RV64IBB-NEXT:    orc.b a0, a0
+; RV64IBB-NEXT:    ret
+  %tmp = call i64 @llvm.riscv.orc.b.i64(i64 %a)
+ ret i64 %tmp
+}