[clang] [llvm] [RISCV] Implement Clang Builtins for XCValu Extension in CV32E40P (PR #100684)

2024-08-16 Thread Jeremy Bennett via cfe-commits


@@ -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)

2024-08-16 Thread Jeremy Bennett via cfe-commits


@@ -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)

2024-08-16 Thread Jeremy Bennett via cfe-commits


@@ -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)

2024-08-02 Thread Jeremy Bennett via cfe-commits


@@ -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