[PATCH for-9.0] tcg/optimize: Fix sign_mask for logical right-shift

[PATCH for-9.0] tcg/optimize: Fix sign_mask for logical right-shift

[Richard Henderson]

The 'sign' computation is attempting to locate the sign bit that has
been repeated, so that we can test if that bit is known zero.  That
computation can be zero if there are no known sign repetitions.

Cc: qemu-stable@nongnu.org
Fixes: 93a967fbb57 ("tcg/optimize: Propagate sign info for shifting")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2248
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/optimize.c                    |  2 +-
 tests/tcg/aarch64/test-2248.c     | 25 +++++++++++++++++++++++++
 tests/tcg/aarch64/Makefile.target |  1 +
 3 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/aarch64/test-2248.c

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 752cc5c56b..275db77b42 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -2376,7 +2376,7 @@ static bool fold_shift(OptContext *ctx, TCGOp *op)
          * will not reduced the number of input sign repetitions.
          */
         sign = (s_mask & -s_mask) >> 1;
-        if (!(z_mask & sign)) {
+        if (sign && !(z_mask & sign)) {
             ctx->s_mask = s_mask;
         }
         break;
diff --git a/tests/tcg/aarch64/test-2248.c b/tests/tcg/aarch64/test-2248.c
new file mode 100644
index 0000000000..6cc20e3c6c
--- /dev/null
+++ b/tests/tcg/aarch64/test-2248.c
@@ -0,0 +1,25 @@
+#include <assert.h>
+
+__attribute__((noinline))
+long test(long x, long y, long sh)
+{
+    long r;
+    asm("cmp   %1, %2\n\t"
+        "cset  x12, lt\n\t"
+        "and   w11, w12, #0xff\n\t"
+        "cmp   w11, #0\n\t"
+        "csetm x14, ne\n\t"
+        "lsr   x13, x14, %3\n\t"
+        "sxtb  %0, w13"
+        : "=r"(r)
+        : "r"(x), "r"(y), "r"(sh)
+        : "x11", "x12", "x13", "x14");
+    return r;
+}
+
+int main()
+{
+    long r = test(0, 1, 2);
+    assert(r == -1);
+    return 0;
+}
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index ea3e232e65..0efd565f05 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -10,6 +10,7 @@ VPATH 		+= $(AARCH64_SRC)
 
 # Base architecture tests
 AARCH64_TESTS=fcvt pcalign-a64 lse2-fault
+AARCH64_TESTS += test-2248
 
 fcvt: LDFLAGS+=-lm
 
-- 
2.34.1

Re: [PATCH for-9.0] tcg/optimize: Fix sign_mask for logical right-shift

[Philippe Mathieu-Daudé]

On 26/3/24 23:22, Richard Henderson wrote:
> The 'sign' computation is attempting to locate the sign bit that has
> been repeated, so that we can test if that bit is known zero.  That
> computation can be zero if there are no known sign repetitions.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 93a967fbb57 ("tcg/optimize: Propagate sign info for shifting")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2248
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/optimize.c                    |  2 +-
>   tests/tcg/aarch64/test-2248.c     | 25 +++++++++++++++++++++++++
>   tests/tcg/aarch64/Makefile.target |  1 +
>   3 files changed, 27 insertions(+), 1 deletion(-)
>   create mode 100644 tests/tcg/aarch64/test-2248.c


> diff --git a/tests/tcg/aarch64/test-2248.c b/tests/tcg/aarch64/test-2248.c
> new file mode 100644
> index 0000000000..6cc20e3c6c
> --- /dev/null
> +++ b/tests/tcg/aarch64/test-2248.c
> @@ -0,0 +1,25 @@

   /* SPDX-License-Identifier: GPL-2.0-or-later */

   /* See https://gitlab.com/qemu-project/qemu/-/issues/2248 */

> +#include <assert.h>
> +
> +__attribute__((noinline))
> +long test(long x, long y, long sh)
> +{
> +    long r;
> +    asm("cmp   %1, %2\n\t"
> +        "cset  x12, lt\n\t"
> +        "and   w11, w12, #0xff\n\t"
> +        "cmp   w11, #0\n\t"
> +        "csetm x14, ne\n\t"
> +        "lsr   x13, x14, %3\n\t"
> +        "sxtb  %0, w13"
> +        : "=r"(r)
> +        : "r"(x), "r"(y), "r"(sh)
> +        : "x11", "x12", "x13", "x14");
> +    return r;
> +}
> +
> +int main()
> +{
> +    long r = test(0, 1, 2);
> +    assert(r == -1);
> +    return 0;
> +}

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>