[PATCH] D99320: [RISCV] [1/2] Add intrinsic for Zbb extension

Craig Topper via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 29 10:57:42 PDT 2021


craig.topper added inline comments.


================
Comment at: clang/include/clang/Basic/BuiltinsRISCV.def:21
+// Zbb extension
+TARGET_BUILTIN(__builtin_riscv_orc_b, "LiLi", "nc", "experimental-zbb")
+TARGET_BUILTIN(__builtin_riscv32_orc_b, "ZiZi", "nc", "experimental-zbb")
----------------
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?


================
Comment at: clang/include/clang/Basic/BuiltinsRISCV.def:22
+TARGET_BUILTIN(__builtin_riscv_orc_b, "LiLi", "nc", "experimental-zbb")
+TARGET_BUILTIN(__builtin_riscv32_orc_b, "ZiZi", "nc", "experimental-zbb")
+TARGET_BUILTIN(__builtin_riscv64_orc_b, "WiWi", "nc", "experimental-zbb")
----------------
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.


================
Comment at: clang/include/clang/Basic/BuiltinsRISCV.def:23
+TARGET_BUILTIN(__builtin_riscv32_orc_b, "ZiZi", "nc", "experimental-zbb")
+TARGET_BUILTIN(__builtin_riscv64_orc_b, "WiWi", "nc", "experimental-zbb")
+
----------------
Ideally this would be "experimental-zbb,64bit" but I'm not sure that the frontend knows about "64bit".


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4175
+    case Intrinsic::riscv_orc_b: {
+      SDValue LHS =
+          DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, N->getOperand(0));
----------------
"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);
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99320/new/

https://reviews.llvm.org/D99320



More information about the cfe-commits mailing list