Re: [PATCH] i386: Fix wrong codegen for -mrelax-cmpxchg-loop
On Thu, Nov 18, 2021 at 8:37 AM Hongyu Wang wrote:
>
> Hi Uros,
>
> For -mrelax-cmpxchg-loop introduced by PR 103069/r12-5265, it would
> produce infinite loop. The correct code should be
>
> .L84:
> movl(%rdi), %ecx
> movl%eax, %edx
> orl %esi, %edx
> cmpl%eax, %ecx
> jne .L82
> lock cmpxchgl %edx, (%rdi)
> jne .L84
> movl%r8d, %eax <<< retval is missing in previous impl
> ret
> .L82:
> rep nop
> jmp .L84
>
> Adjust corresponding expander to fix such issue, and fix runtime test
> so the problem would be exposed.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for master?
>
> gcc/ChangeLog:
>
> * config/i386/i386-expand.c (ix86_expand_atomic_fetch_op_loop):
> Adjust generated cfg to avoid infinite loop.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr103069-2.c: Adjust.
LGTM.
Thanks,
Uros.
> ---
> gcc/config/i386/i386-expand.c | 7 ++-
> gcc/testsuite/gcc.target/i386/pr103069-2.c | 11 ++-
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c
> index 3e4de64ec24..0d5d1a0e205 100644
> --- a/gcc/config/i386/i386-expand.c
> +++ b/gcc/config/i386/i386-expand.c
> @@ -23143,13 +23143,14 @@ void ix86_expand_atomic_fetch_op_loop (rtx target,
> rtx mem, rtx val,
>bool doubleword)
> {
>rtx old_reg, new_reg, old_mem, success, oldval, new_mem;
> - rtx_code_label *loop_label, *pause_label;
> + rtx_code_label *loop_label, *pause_label, *done_label;
>machine_mode mode = GET_MODE (target);
>
>old_reg = gen_reg_rtx (mode);
>new_reg = old_reg;
>loop_label = gen_label_rtx ();
>pause_label = gen_label_rtx ();
> + done_label = gen_label_rtx ();
>old_mem = copy_to_reg (mem);
>emit_label (loop_label);
>emit_move_insn (old_reg, old_mem);
> @@ -23207,11 +23208,15 @@ void ix86_expand_atomic_fetch_op_loop (rtx target,
> rtx mem, rtx val,
>GET_MODE (success), 1, loop_label,
>profile_probability::guessed_never ());
>
> + emit_jump_insn (gen_jump (done_label));
> + emit_barrier ();
> +
>/* If mem is not expected, pause and loop back. */
>emit_label (pause_label);
>emit_insn (gen_pause ());
>emit_jump_insn (gen_jump (loop_label));
>emit_barrier ();
> + emit_label (done_label);
> }
>
> #include "gt-i386-expand.h"
> diff --git a/gcc/testsuite/gcc.target/i386/pr103069-2.c
> b/gcc/testsuite/gcc.target/i386/pr103069-2.c
> index 8ac824cc8e8..b3f2235fd55 100644
> --- a/gcc/testsuite/gcc.target/i386/pr103069-2.c
> +++ b/gcc/testsuite/gcc.target/i386/pr103069-2.c
> @@ -1,5 +1,5 @@
> -/* PR target/103068 */
> -/* { dg-do compile } */
> +/* PR target/103069 */
> +/* { dg-do run } */
> /* { dg-additional-options "-O2 -march=x86-64 -mtune=generic" } */
>
> #include
> @@ -37,13 +37,14 @@ FUNC_ATOMIC_RELAX (char, xor)
> #define TEST_ATOMIC_FETCH_LOGIC(TYPE, OP) \
> { \
>TYPE a = 11, b = 101, res, exp; \
> + TYPE c = 11, d = 101;\
>res = relax_##TYPE##_##OP##_fetch (, b); \
> - exp = f_##TYPE##_##OP##_fetch (, b); \
> + exp = f_##TYPE##_##OP##_fetch (, d); \
>if (res != exp) \
> abort (); \
> - a = 21, b = 92; \
> + a = c = 21, b = d = 92; \
>res = relax_##TYPE##_fetch_##OP (, b); \
> - exp = f_##TYPE##_fetch_##OP (, b); \
> + exp = f_##TYPE##_fetch_##OP (, d); \
>if (res != exp) \
> abort (); \
> }
> --
> 2.18.1
>
[PATCH] i386: Fix wrong codegen for -mrelax-cmpxchg-loop
Hi Uros,
For -mrelax-cmpxchg-loop introduced by PR 103069/r12-5265, it would
produce infinite loop. The correct code should be
.L84:
movl(%rdi), %ecx
movl%eax, %edx
orl %esi, %edx
cmpl%eax, %ecx
jne .L82
lock cmpxchgl %edx, (%rdi)
jne .L84
movl%r8d, %eax <<< retval is missing in previous impl
ret
.L82:
rep nop
jmp .L84
Adjust corresponding expander to fix such issue, and fix runtime test
so the problem would be exposed.
Bootstrapped/regtested on x86_64-pc-linux-gnu{-m32,}.
Ok for master?
gcc/ChangeLog:
* config/i386/i386-expand.c (ix86_expand_atomic_fetch_op_loop):
Adjust generated cfg to avoid infinite loop.
gcc/testsuite/ChangeLog:
* gcc.target/i386/pr103069-2.c: Adjust.
---
gcc/config/i386/i386-expand.c | 7 ++-
gcc/testsuite/gcc.target/i386/pr103069-2.c | 11 ++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c
index 3e4de64ec24..0d5d1a0e205 100644
--- a/gcc/config/i386/i386-expand.c
+++ b/gcc/config/i386/i386-expand.c
@@ -23143,13 +23143,14 @@ void ix86_expand_atomic_fetch_op_loop (rtx target,
rtx mem, rtx val,
bool doubleword)
{
rtx old_reg, new_reg, old_mem, success, oldval, new_mem;
- rtx_code_label *loop_label, *pause_label;
+ rtx_code_label *loop_label, *pause_label, *done_label;
machine_mode mode = GET_MODE (target);
old_reg = gen_reg_rtx (mode);
new_reg = old_reg;
loop_label = gen_label_rtx ();
pause_label = gen_label_rtx ();
+ done_label = gen_label_rtx ();
old_mem = copy_to_reg (mem);
emit_label (loop_label);
emit_move_insn (old_reg, old_mem);
@@ -23207,11 +23208,15 @@ void ix86_expand_atomic_fetch_op_loop (rtx target,
rtx mem, rtx val,
GET_MODE (success), 1, loop_label,
profile_probability::guessed_never ());
+ emit_jump_insn (gen_jump (done_label));
+ emit_barrier ();
+
/* If mem is not expected, pause and loop back. */
emit_label (pause_label);
emit_insn (gen_pause ());
emit_jump_insn (gen_jump (loop_label));
emit_barrier ();
+ emit_label (done_label);
}
#include "gt-i386-expand.h"
diff --git a/gcc/testsuite/gcc.target/i386/pr103069-2.c
b/gcc/testsuite/gcc.target/i386/pr103069-2.c
index 8ac824cc8e8..b3f2235fd55 100644
--- a/gcc/testsuite/gcc.target/i386/pr103069-2.c
+++ b/gcc/testsuite/gcc.target/i386/pr103069-2.c
@@ -1,5 +1,5 @@
-/* PR target/103068 */
-/* { dg-do compile } */
+/* PR target/103069 */
+/* { dg-do run } */
/* { dg-additional-options "-O2 -march=x86-64 -mtune=generic" } */
#include
@@ -37,13 +37,14 @@ FUNC_ATOMIC_RELAX (char, xor)
#define TEST_ATOMIC_FETCH_LOGIC(TYPE, OP) \
{ \
TYPE a = 11, b = 101, res, exp; \
+ TYPE c = 11, d = 101;\
res = relax_##TYPE##_##OP##_fetch (, b); \
- exp = f_##TYPE##_##OP##_fetch (, b); \
+ exp = f_##TYPE##_##OP##_fetch (, d); \
if (res != exp) \
abort (); \
- a = 21, b = 92; \
+ a = c = 21, b = d = 92; \
res = relax_##TYPE##_fetch_##OP (, b); \
- exp = f_##TYPE##_fetch_##OP (, b); \
+ exp = f_##TYPE##_fetch_##OP (, d); \
if (res != exp) \
abort (); \
}
--
2.18.1
