[clang] [llvm] [RISCV] Implement Clang Builtins for XCValu Extension in CV32E40P (PR #100684)
@@ -59,16 +59,26 @@ let TargetPrefix = "riscv" in { [IntrNoMem, IntrWillReturn, IntrSpeculatable, ImmArg>, ImmArg>]>; + def int_riscv_cv_alu_slet : ScalarCoreVAluGprGprIntrinsic; jeremybennett wrote: > > Hi @topperc > > There are a few things going on here. @topperc Thanks for this. Apologies if my original comment was a bit clumsy. I was trying to explain where we were, not where we should go! > > 2. These instructions only exist for "less than or equal", there are no > > equivalents for other comparison predicates, hence just these two builtins > > mapping to the instructions. > > I was referring to the stand comparisons. We don't have builtins for writing > slt, we expect the programmer to write (a< b) and the backend will figure it > out. slte can be automatically selected from (a<=b). Why do we need a builtin > too? I understand. > > 3. LLVM and GCC for CORE-V are intended to be equivalent, and in GCC, > > having the builtin to generate the instruction is more efficient than using > > inline assembler. First the dataflow through the instruction is explicit, > > secondly the pattern for the functionality is exposed to GCC, potentially > > allowing the instruction to be generated automatically in other > > circumstances. > > Since gcc already has the builtin, I guess we have to be source compatible. > Would it be ok to map the builtin to (zext (icmp sle a,b) in IR and let the > middle end and backend treat it like any other compare? This is fine. We just want to make sure that any legacy code written for GCC is not too painful to switch to LLVM for the owners. https://github.com/llvm/llvm-project/pull/100684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [RISCV] Implement Clang Builtins for XCValu Extension in CV32E40P (PR #100684)
@@ -59,16 +59,26 @@ let TargetPrefix = "riscv" in { [IntrNoMem, IntrWillReturn, IntrSpeculatable, ImmArg>, ImmArg>]>; + def int_riscv_cv_alu_slet : ScalarCoreVAluGprGprIntrinsic; jeremybennett wrote: > Is it too late to remove the builtin? I can't imagine there's much software > out there actually using these that would be a pain to change. @jrtc27 Thanks for the feedback. The problem with an open source core, is that you don't know who has picked it up and adopted it. This core is particularly attractive because of its verification status. I have come across one very large multinational (not an OpenHW member) using the core, without having made any public declaration that this is the core they are using. We could try dropping it and see if anyone complains. https://github.com/llvm/llvm-project/pull/100684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [RISCV] Implement Clang Builtins for XCValu Extension in CV32E40P (PR #100684)
@@ -59,16 +59,26 @@ let TargetPrefix = "riscv" in { [IntrNoMem, IntrWillReturn, IntrSpeculatable, ImmArg>, ImmArg>]>; + def int_riscv_cv_alu_slet : ScalarCoreVAluGprGprIntrinsic; jeremybennett wrote: > I also note that conversations like this is why standard RISC-V builtins go > through a review process seen by developers of both toolchains. I would > encourage CORE-V to do the same if they don't want to be surprised by > pushback for things that developers of certain toolchains think don't make > sense. @jrtc27 We have to put our hands up on this one. For a long time the CORE-V and its ancestors have only had GCC tool chains, so the engagement was with that community. The LLVM implementation is newer, but we should have reached out early in the project. How would you suggest we progress given where we are? https://github.com/llvm/llvm-project/pull/100684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [RISCV] Implement Clang Builtins for XCValu Extension in CV32E40P (PR #100684)
@@ -59,16 +59,26 @@ let TargetPrefix = "riscv" in { [IntrNoMem, IntrWillReturn, IntrSpeculatable, ImmArg>, ImmArg>]>; + def int_riscv_cv_alu_slet : ScalarCoreVAluGprGprIntrinsic; jeremybennett wrote: Hi @topperc There are a few things going on here. 1. The naming of the corresponding instructions in the CORE-V ISA extension was changed from `slet` to `sle`, and `sletu` to `sleu`. The CORE-V [standard for builtins](https://github.com/openhwgroup/core-v-sw/blob/master/specifications/corev-builtin-spec.md) has not yet been updated to reflect this change. This needs to be fixed through OpenHW group's standardization processes (@PaoloS02 is leading this inside OpenHW Group). 2. These instructions only exist for "less than or equal", there are no equivalents for other comparison predicates, hence just these two builtins mapping to the instructions. 3. LLVM and GCC for CORE-V are intended to be equivalent, and in GCC, having the builtin to generate the instruction is more efficient than using inline assembler. First the dataflow through the instruction is explicit, secondly the pattern for the functionality is exposed to GCC, potentially allowing the instruction to be generated automatically in other circumstances. So in summary i) it's there because the CORE-V standard for builtins says it should be and ii) it's there to match GCC behavior. Hope this is helpful. https://github.com/llvm/llvm-project/pull/100684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits