bpf: fix memory constraint of ldx/stx instructions [PR108790]

Message ID 20230214195153.8445-1-david.faust@oracle.com
State Accepted
Headers
Series bpf: fix memory constraint of ldx/stx instructions [PR108790] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

David Faust Feb. 14, 2023, 7:51 p.m. UTC
  In some cases where the target memory address for an ldx or stx
instruction could be reduced to a constant, GCC could emit a malformed
instruction like:

    ldxdw %r0,0

Rather than the expected form:

    ldxdw %rX, [%rY + OFFSET]

This is due to the constraint allowing a const_int operand, which the
output templates do not handle.

Fix it by introducing a new memory constraint for the appropriate
operands of these instructions, which is identical to 'm' except that
it does not accept const_int.

Tested with bpf-unknown-none, no known regressions.
OK?

Thanks.

gcc/

	PR target/108790
	* config/bpf/constraints.md (q): New memory constraint.
	* config/bpf/bpf.md (zero_extendhidi2): Use it here.
	(zero_extendqidi2): Likewise.
	(zero_extendsidi2): Likewise.
	(*mov<MM:mode>): Likewise.

gcc/testsuite/

	PR target/108790
	* gcc.target/bpf/ldxdw.c: New test.
---
 gcc/config/bpf/bpf.md                | 10 +++++-----
 gcc/config/bpf/constraints.md        | 11 +++++++++++
 gcc/testsuite/gcc.target/bpf/ldxdw.c | 12 ++++++++++++
 3 files changed, 28 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/bpf/ldxdw.c
  

Comments

Jose E. Marchesi Feb. 14, 2023, 10:33 p.m. UTC | #1
Hi David.

> In some cases where the target memory address for an ldx or stx
> instruction could be reduced to a constant, GCC could emit a malformed
> instruction like:
>
>     ldxdw %r0,0
>
> Rather than the expected form:
>
>     ldxdw %rX, [%rY + OFFSET]
>
> This is due to the constraint allowing a const_int operand, which the
> output templates do not handle.
>
> Fix it by introducing a new memory constraint for the appropriate
> operands of these instructions, which is identical to 'm' except that
> it does not accept const_int.
>
> Tested with bpf-unknown-none, no known regressions.
> OK?

OK.  Thanks for the patch.

> Thanks.
>
> gcc/
>
> 	PR target/108790
> 	* config/bpf/constraints.md (q): New memory constraint.
> 	* config/bpf/bpf.md (zero_extendhidi2): Use it here.
> 	(zero_extendqidi2): Likewise.
> 	(zero_extendsidi2): Likewise.
> 	(*mov<MM:mode>): Likewise.
>
> gcc/testsuite/
>
> 	PR target/108790
> 	* gcc.target/bpf/ldxdw.c: New test.
> ---
>  gcc/config/bpf/bpf.md                | 10 +++++-----
>  gcc/config/bpf/constraints.md        | 11 +++++++++++
>  gcc/testsuite/gcc.target/bpf/ldxdw.c | 12 ++++++++++++
>  3 files changed, 28 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/bpf/ldxdw.c
>
> diff --git a/gcc/config/bpf/bpf.md b/gcc/config/bpf/bpf.md
> index d9af98384ef..f6be0a21234 100644
> --- a/gcc/config/bpf/bpf.md
> +++ b/gcc/config/bpf/bpf.md
> @@ -242,7 +242,7 @@ (define_insn "xor<AM:mode>3"
>  
>  (define_insn "zero_extendhidi2"
>    [(set (match_operand:DI 0 "register_operand" "=r,r,r")
> -	(zero_extend:DI (match_operand:HI 1 "nonimmediate_operand" "0,r,m")))]
> +	(zero_extend:DI (match_operand:HI 1 "nonimmediate_operand" "0,r,q")))]
>    ""
>    "@
>     and\t%0,0xffff
> @@ -252,7 +252,7 @@ (define_insn "zero_extendhidi2"
>  
>  (define_insn "zero_extendqidi2"
>    [(set (match_operand:DI 0 "register_operand" "=r,r,r")
> -	(zero_extend:DI (match_operand:QI 1 "nonimmediate_operand" "0,r,m")))]
> +	(zero_extend:DI (match_operand:QI 1 "nonimmediate_operand" "0,r,q")))]
>    ""
>    "@
>     and\t%0,0xff
> @@ -263,7 +263,7 @@ (define_insn "zero_extendqidi2"
>  (define_insn "zero_extendsidi2"
>    [(set (match_operand:DI 0 "register_operand" "=r,r")
>  	(zero_extend:DI
> -	  (match_operand:SI 1 "nonimmediate_operand" "r,m")))]
> +	  (match_operand:SI 1 "nonimmediate_operand" "r,q")))]
>    ""
>    "@
>     * return bpf_has_alu32 ? \"mov32\t%0,%1\" : \"mov\t%0,%1\;and\t%0,0xffffffff\";
> @@ -302,8 +302,8 @@ (define_expand "mov<MM:mode>"
>  }")
>  
>  (define_insn "*mov<MM:mode>"
> -  [(set (match_operand:MM 0 "nonimmediate_operand" "=r, r,r,m,m")
> -        (match_operand:MM 1 "mov_src_operand"      " m,rI,B,r,I"))]
> +  [(set (match_operand:MM 0 "nonimmediate_operand" "=r, r,r,q,q")
> +        (match_operand:MM 1 "mov_src_operand"      " q,rI,B,r,I"))]
>    ""
>    "@
>     ldx<mop>\t%0,%1
> diff --git a/gcc/config/bpf/constraints.md b/gcc/config/bpf/constraints.md
> index c8a65cfcddb..33f9177b8eb 100644
> --- a/gcc/config/bpf/constraints.md
> +++ b/gcc/config/bpf/constraints.md
> @@ -29,3 +29,14 @@ (define_constraint "B"
>  (define_constraint "S"
>    "A constant call address."
>    (match_code "const,symbol_ref,label_ref,const_int"))
> +
> +;;
> +;; Memory constraints.
> +;;
> +
> +; Just like 'm' but disallows const_int.
> +; Used for ldx[b,h,w,dw] and stx[b,h,w,dw] instructions.
> +(define_memory_constraint "q"
> +  "Memory reference which is not a constant integer."
> +  (and (match_code "mem")
> +       (match_test "GET_CODE(XEXP(op, 0)) != CONST_INT")))
> diff --git a/gcc/testsuite/gcc.target/bpf/ldxdw.c b/gcc/testsuite/gcc.target/bpf/ldxdw.c
> new file mode 100644
> index 00000000000..0985ea3e6ac
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/ldxdw.c
> @@ -0,0 +1,12 @@
> +/* Verify that we do not generate a malformed ldxdw instruction
> +   with a constant instead of register + offset.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +/* { dg-final { scan-assembler-times "ldxdw\t%r.,\\\[%r.+0\\\]" 1 } } */
> +/* { dg-final { scan-assembler-not "ldxdw\t%r.,\[0-9\]+" } } */
> +
> +unsigned long long test () {
> +  return *((unsigned long long *) 0x4000);
> +}
  

Patch

diff --git a/gcc/config/bpf/bpf.md b/gcc/config/bpf/bpf.md
index d9af98384ef..f6be0a21234 100644
--- a/gcc/config/bpf/bpf.md
+++ b/gcc/config/bpf/bpf.md
@@ -242,7 +242,7 @@  (define_insn "xor<AM:mode>3"
 
 (define_insn "zero_extendhidi2"
   [(set (match_operand:DI 0 "register_operand" "=r,r,r")
-	(zero_extend:DI (match_operand:HI 1 "nonimmediate_operand" "0,r,m")))]
+	(zero_extend:DI (match_operand:HI 1 "nonimmediate_operand" "0,r,q")))]
   ""
   "@
    and\t%0,0xffff
@@ -252,7 +252,7 @@  (define_insn "zero_extendhidi2"
 
 (define_insn "zero_extendqidi2"
   [(set (match_operand:DI 0 "register_operand" "=r,r,r")
-	(zero_extend:DI (match_operand:QI 1 "nonimmediate_operand" "0,r,m")))]
+	(zero_extend:DI (match_operand:QI 1 "nonimmediate_operand" "0,r,q")))]
   ""
   "@
    and\t%0,0xff
@@ -263,7 +263,7 @@  (define_insn "zero_extendqidi2"
 (define_insn "zero_extendsidi2"
   [(set (match_operand:DI 0 "register_operand" "=r,r")
 	(zero_extend:DI
-	  (match_operand:SI 1 "nonimmediate_operand" "r,m")))]
+	  (match_operand:SI 1 "nonimmediate_operand" "r,q")))]
   ""
   "@
    * return bpf_has_alu32 ? \"mov32\t%0,%1\" : \"mov\t%0,%1\;and\t%0,0xffffffff\";
@@ -302,8 +302,8 @@  (define_expand "mov<MM:mode>"
 }")
 
 (define_insn "*mov<MM:mode>"
-  [(set (match_operand:MM 0 "nonimmediate_operand" "=r, r,r,m,m")
-        (match_operand:MM 1 "mov_src_operand"      " m,rI,B,r,I"))]
+  [(set (match_operand:MM 0 "nonimmediate_operand" "=r, r,r,q,q")
+        (match_operand:MM 1 "mov_src_operand"      " q,rI,B,r,I"))]
   ""
   "@
    ldx<mop>\t%0,%1
diff --git a/gcc/config/bpf/constraints.md b/gcc/config/bpf/constraints.md
index c8a65cfcddb..33f9177b8eb 100644
--- a/gcc/config/bpf/constraints.md
+++ b/gcc/config/bpf/constraints.md
@@ -29,3 +29,14 @@  (define_constraint "B"
 (define_constraint "S"
   "A constant call address."
   (match_code "const,symbol_ref,label_ref,const_int"))
+
+;;
+;; Memory constraints.
+;;
+
+; Just like 'm' but disallows const_int.
+; Used for ldx[b,h,w,dw] and stx[b,h,w,dw] instructions.
+(define_memory_constraint "q"
+  "Memory reference which is not a constant integer."
+  (and (match_code "mem")
+       (match_test "GET_CODE(XEXP(op, 0)) != CONST_INT")))
diff --git a/gcc/testsuite/gcc.target/bpf/ldxdw.c b/gcc/testsuite/gcc.target/bpf/ldxdw.c
new file mode 100644
index 00000000000..0985ea3e6ac
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/ldxdw.c
@@ -0,0 +1,12 @@ 
+/* Verify that we do not generate a malformed ldxdw instruction
+   with a constant instead of register + offset.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+/* { dg-final { scan-assembler-times "ldxdw\t%r.,\\\[%r.+0\\\]" 1 } } */
+/* { dg-final { scan-assembler-not "ldxdw\t%r.,\[0-9\]+" } } */
+
+unsigned long long test () {
+  return *((unsigned long long *) 0x4000);
+}