diff mbox series

bpf: use xBPF signed div, mod insns when available

Message ID 20200917171530.37619-1-david.faust@oracle.com
State New
Headers show
Series bpf: use xBPF signed div, mod insns when available | expand

Commit Message

David Faust Sept. 17, 2020, 5:15 p.m. UTC 
The 'mod' and 'div' operators in eBPF are unsigned, with no signed
counterpart. xBPF adds two new ALU operations, sdiv and smod, for
signed division and modulus, respectively. Update bpf.md with
'define_insn' blocks for signed div and mod to use them when targetting
xBPF, and add new tests to ensure they are used appropriately.

2020-09-17  David Faust  <david.faust@oracle.com>

gcc/
	* config/bpf/bpf.md: Add defines for signed div and mod operators.

gcc/testsuite/
	* gcc.target/bpf/diag-sdiv.c: New test.
	* gcc.target/bpf/diag-smod.c: New test.
	* gcc.target/bpf/xbpf-sdiv-1.c: New test.
	* gcc.target/bpf/xbpf-smod-1.c: New test.
---
 gcc/config/bpf/bpf.md                      | 20 ++++++++++++++++++++
 gcc/testsuite/gcc.target/bpf/diag-sdiv.c   | 12 ++++++++++++
 gcc/testsuite/gcc.target/bpf/diag-smod.c   | 12 ++++++++++++
 gcc/testsuite/gcc.target/bpf/xbpf-sdiv-1.c | 14 ++++++++++++++
 gcc/testsuite/gcc.target/bpf/xbpf-smod-1.c | 14 ++++++++++++++
 5 files changed, 72 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/bpf/diag-sdiv.c
 create mode 100644 gcc/testsuite/gcc.target/bpf/diag-smod.c
 create mode 100644 gcc/testsuite/gcc.target/bpf/xbpf-sdiv-1.c
 create mode 100644 gcc/testsuite/gcc.target/bpf/xbpf-smod-1.c

Comments

Jeff Law Sept. 17, 2020, 10:51 p.m. UTC | #1 
On 9/17/20 11:15 AM, David Faust via Gcc-patches wrote:
> The 'mod' and 'div' operators in eBPF are unsigned, with no signed
> counterpart. xBPF adds two new ALU operations, sdiv and smod, for
> signed division and modulus, respectively. Update bpf.md with
> 'define_insn' blocks for signed div and mod to use them when targetting
> xBPF, and add new tests to ensure they are used appropriately.
>
> 2020-09-17  David Faust  <david.faust@oracle.com>
>
> gcc/
> 	* config/bpf/bpf.md: Add defines for signed div and mod operators.
>
> gcc/testsuite/
> 	* gcc.target/bpf/diag-sdiv.c: New test.
> 	* gcc.target/bpf/diag-smod.c: New test.
> 	* gcc.target/bpf/xbpf-sdiv-1.c: New test.
> 	* gcc.target/bpf/xbpf-smod-1.c: New test.

OK.  But give Jose 48hrs before committing just in case there's
something he wants to comment on.  Y'all are far more familiar with bpf
than I ;-)


jeff
Jose E. Marchesi Sept. 18, 2020, 3:01 p.m. UTC | #2 
>> The 'mod' and 'div' operators in eBPF are unsigned, with no signed
>> counterpart. xBPF adds two new ALU operations, sdiv and smod, for
>> signed division and modulus, respectively. Update bpf.md with
>> 'define_insn' blocks for signed div and mod to use them when targetting
>> xBPF, and add new tests to ensure they are used appropriately.
>>
>> 2020-09-17  David Faust  <david.faust@oracle.com>
>>
>> gcc/
>> 	* config/bpf/bpf.md: Add defines for signed div and mod operators.
>>
>> gcc/testsuite/
>> 	* gcc.target/bpf/diag-sdiv.c: New test.
>> 	* gcc.target/bpf/diag-smod.c: New test.
>> 	* gcc.target/bpf/xbpf-sdiv-1.c: New test.
>> 	* gcc.target/bpf/xbpf-smod-1.c: New test.
>
> OK.  But give Jose 48hrs before committing just in case there's
> something he wants to comment on.  Y'all are far more familiar with bpf
> than I ;-)

Looks good to me! :)
But the related pending patch in binutils should go in first.
David Faust Sept. 18, 2020, 5:12 p.m. UTC | #3 
> 
>>> The 'mod' and 'div' operators in eBPF are unsigned, with no signed
>>> counterpart. xBPF adds two new ALU operations, sdiv and smod, for
>>> signed division and modulus, respectively. Update bpf.md with
>>> 'define_insn' blocks for signed div and mod to use them when targetting
>>> xBPF, and add new tests to ensure they are used appropriately.
>>>
>>> 2020-09-17  David Faust  <david.faust@oracle.com>
>>>
>>> gcc/
>>> 	* config/bpf/bpf.md: Add defines for signed div and mod operators.
>>>
>>> gcc/testsuite/
>>> 	* gcc.target/bpf/diag-sdiv.c: New test.
>>> 	* gcc.target/bpf/diag-smod.c: New test.
>>> 	* gcc.target/bpf/xbpf-sdiv-1.c: New test.
>>> 	* gcc.target/bpf/xbpf-smod-1.c: New test.
>>
>> OK.  But give Jose 48hrs before committing just in case there's
>> something he wants to comment on.  Y'all are far more familiar with bpf
>> than I ;-)
> 
> Looks good to me! :)
> But the related pending patch in binutils should go in first.
> 
Hi!

I just checked in the binutils patch, but I don't actually have write
access for gcc.

If someone could check this in for me I'd appreciate it :)

Thanks!
David
Segher Boessenkool Sept. 18, 2020, 11:15 p.m. UTC | #4 
Hi!

On Thu, Sep 17, 2020 at 10:15:30AM -0700, David Faust via Gcc-patches wrote:
> The 'mod' and 'div' operators in eBPF are unsigned, with no signed
> counterpart. xBPF adds two new ALU operations, sdiv and smod, for
> signed division and modulus, respectively. Update bpf.md with
> 'define_insn' blocks for signed div and mod to use them when targetting
> xBPF, and add new tests to ensure they are used appropriately.

So why does xBPF have signed versions of the divides?  Is it because it
is wanted to have it in eBPF eventually?  Is it because the libgcc
routines are just too slow?  Is it because (the generic) libgcc does not
trap for MIN_INT / -1 ?  Some other reason?

(I'm just curious; I cannot figure it out :-) )


Segher
Jose E. Marchesi Sept. 22, 2020, 6:13 p.m. UTC | #5 
Hi Segher!

> On Thu, Sep 17, 2020 at 10:15:30AM -0700, David Faust via Gcc-patches wrote:
>> The 'mod' and 'div' operators in eBPF are unsigned, with no signed
>> counterpart. xBPF adds two new ALU operations, sdiv and smod, for
>> signed division and modulus, respectively. Update bpf.md with
>> 'define_insn' blocks for signed div and mod to use them when targetting
>> xBPF, and add new tests to ensure they are used appropriately.
>
> So why does xBPF have signed versions of the divides?  Is it because it
> is wanted to have it in eBPF eventually?  Is it because the libgcc
> routines are just too slow?  Is it because (the generic) libgcc does not
> trap for MIN_INT / -1 ?  Some other reason?
>
> (I'm just curious; I cannot figure it out :-) )

I don't know if eBPF will be adopting signed division instructions at
some point... judging from what can be read in their documentation, most
probably they will not.

In xBPF we mainly want to avoid the funcall.  Linking of BPF objects is
still an... eer fuzzy area.  Also, we reckon some potential applications
of xbpf (like using it instead of dwarf in the Infinity project) may
find it simpler to have the instructions than to rely on a software
implementation like libgcc's.
Jose E. Marchesi Sept. 22, 2020, 6:39 p.m. UTC | #6 
>>>> The 'mod' and 'div' operators in eBPF are unsigned, with no signed
>>>> counterpart. xBPF adds two new ALU operations, sdiv and smod, for
>>>> signed division and modulus, respectively. Update bpf.md with
>>>> 'define_insn' blocks for signed div and mod to use them when targetting
>>>> xBPF, and add new tests to ensure they are used appropriately.
>>>>
>>>> 2020-09-17  David Faust  <david.faust@oracle.com>
>>>>
>>>> gcc/
>>>> 	* config/bpf/bpf.md: Add defines for signed div and mod operators.
>>>>
>>>> gcc/testsuite/
>>>> 	* gcc.target/bpf/diag-sdiv.c: New test.
>>>> 	* gcc.target/bpf/diag-smod.c: New test.
>>>> 	* gcc.target/bpf/xbpf-sdiv-1.c: New test.
>>>> 	* gcc.target/bpf/xbpf-smod-1.c: New test.
>>>
>>> OK.  But give Jose 48hrs before committing just in case there's
>>> something he wants to comment on.  Y'all are far more familiar with bpf
>>> than I ;-)
>> 
>> Looks good to me! :)
>> But the related pending patch in binutils should go in first.
>> 
> Hi!
>
> I just checked in the binutils patch, but I don't actually have write
> access for gcc.

Just did it on your behalf.
Thanks!
diff mbox series

Patch

diff --git a/gcc/config/bpf/bpf.md b/gcc/config/bpf/bpf.md
index 41bb4fcd9a7..ac87e3d9c53 100644
--- a/gcc/config/bpf/bpf.md
+++ b/gcc/config/bpf/bpf.md
@@ -160,6 +160,16 @@  (define_insn "udiv<AM:mode>3"
   "div<msuffix>\t%0,%2"
   [(set_attr "type" "<mtype>")])
 
+;; However, xBPF does provide a signed division operator, sdiv.
+
+(define_insn "div<AM:mode>3"
+  [(set (match_operand:AM 0 "register_operand" "=r,r")
+        (div:AM (match_operand:AM 1 "register_operand" " 0,0")
+                (match_operand:AM 2 "reg_or_imm_operand" "r,I")))]
+  "TARGET_XBPF"
+  "sdiv<msuffix>\t%0,%2"
+  [(set_attr "type" "<mtype>")])
+
 ;;; Modulus
 
 ;; Note that eBPF doesn't provide instructions for signed integer
@@ -173,6 +183,16 @@  (define_insn "umod<AM:mode>3"
   "mod<msuffix>\t%0,%2"
   [(set_attr "type" "<mtype>")])
 
+;; Again, xBPF provides a signed version, smod.
+
+(define_insn "mod<AM:mode>3"
+  [(set (match_operand:AM 0 "register_operand" "=r,r")
+        (mod:AM (match_operand:AM 1 "register_operand" " 0,0")
+                (match_operand:AM 2 "reg_or_imm_operand" "r,I")))]
+  "TARGET_XBPF"
+  "smod<msuffix>\t%0,%2"
+  [(set_attr "type" "<mtype>")])
+
 ;;; Logical AND
 (define_insn "and<AM:mode>3"
   [(set (match_operand:AM 0 "register_operand" "=r,r")
diff --git a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
new file mode 100644
index 00000000000..db0c494a789
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
@@ -0,0 +1,12 @@ 
+/* Verify signed division does not produce 'sdiv' insn in eBPF.  */
+/* { dg-do compile } */
+/* { dg-options "-O0" } */
+
+void
+foo ()
+{
+  signed int x = 5;
+  signed int y = 2;
+  signed int z = x / y;
+}
+/* { dg-final { scan-assembler-not "sdiv(32)?\t%r" } } */
diff --git a/gcc/testsuite/gcc.target/bpf/diag-smod.c b/gcc/testsuite/gcc.target/bpf/diag-smod.c
new file mode 100644
index 00000000000..20234ee39cc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/diag-smod.c
@@ -0,0 +1,12 @@ 
+/* Verify signed modulo does not produce 'smod' insn in eBPF.  */
+/* { dg-do compile } */
+/* { dg-options "-O0" } */
+
+void
+foo ()
+{
+  signed int x = 5;
+  signed int y = 2;
+  signed int z = x % y;
+}
+/* { dg-final { scan-assembler-not "smod(32)?\t%r" } } */
diff --git a/gcc/testsuite/gcc.target/bpf/xbpf-sdiv-1.c b/gcc/testsuite/gcc.target/bpf/xbpf-sdiv-1.c
new file mode 100644
index 00000000000..f6c5c9e9f1c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/xbpf-sdiv-1.c
@@ -0,0 +1,14 @@ 
+/* Verify that sdiv instruction is used for xBPF. */
+/* { dg-do compile } */
+/* { dg-options "-O0 -mxbpf" } */
+
+void
+foo ()
+{
+  signed int x = 5;
+  signed int y = 2;
+  signed int z = x / y;
+  signed int w = x / 3;
+}
+
+/* { dg-final { scan-assembler "sdiv(32)?\t%r" } } */
diff --git a/gcc/testsuite/gcc.target/bpf/xbpf-smod-1.c b/gcc/testsuite/gcc.target/bpf/xbpf-smod-1.c
new file mode 100644
index 00000000000..b3e5816b5cf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/xbpf-smod-1.c
@@ -0,0 +1,14 @@ 
+/* Verify that smod instruction is used for xBPF. */
+/* { dg-do compile } */
+/* { dg-options "-O0 -mxbpf" } */
+
+void
+foo ()
+{
+  signed int x = 5;
+  signed int y = 2;
+  signed int z = x % y;
+  signed int w = x % 3;
+}
+
+/* { dg-final { scan-assembler "smod(32)?\t%r" } } */