i386: Improve code generation of smin(x,0) with -m32.

To make amends for the recent (temporary) testsuite failure
of my new gcc.target/i386/minmax-9.c when compiled with -m32,
this patch improves the -m32 code we generate for the examples
in that test case.

Currently, smax(x,0) generates the very cool implementation:

smax0:  movl    4(%esp), %eax
        cltd
        notl    %edx
        andl    %edx, %eax
        ret

But mysteriously, the related smin(x,0) uses a longer conditional
move sequence:

smin0:  movl    4(%esp), %eax
        xorl    %edx, %edx
        testl   %eax, %eax
        setg    %dl
        subl    $1, %edx
        andl    %edx, %eax
        ret

With this patch, we now generate the much nicer:

smin0:  movl    4(%esp), %eax
        cltd
        andl    %edx, %eax
        ret

The trick is to expand smin(x,0) as "x < 0 ? x : 0" instead
of the current "x <= 0 ? x : 0", as the former can take
advantage of sign_bit_mask operations.

The benefits can also be seen in QImode, where previously:
smin0:  movl    4(%esp), %edx
        testb   %dl, %dl
        setg    %al
        subl    $1, %eax
        andl    %edx, %eax
        ret

now becomes:

smin0:  movl    4(%esp), %edx
        movl    %edx, %eax
        sarb    $7, %al
        andl    %edx, %eax
        ret


This patch has been tested on x86_64-pc-linux-gnu with a
"make bootstrap" and "make -k check" with no new failures.
Please let me know if I need to tweak the dejagnu directives;
no other i386.exp test appears to explicitly specify -m32, but as
a compile test, it's not unreasonable to check things on x86_64.
OK for mainline?


2020-08-10  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	* config/i386/i386-expand.c (ix86_expand_int_movcc): Expand
	signed MIN_EXPR against zero as "x < 0 ? x : 0" instead of
	"x <= 0 ? x : 0" to enable sign_bit_compare_p optimizations.

gcc/testsuite/ChangeLog
	* gcc.target/i386/minmax-12.c: New test.

Thanks in advance,
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK

diff --git a/gcc/testsuite/gcc.target/i386/minmax-12.c b/gcc/testsuite/gcc.target/i386/minmax-12.c
new file mode 100644
index 0000000..bcb3af0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/minmax-12.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-m32 -O2 -march=i386 -mtune=generic" } */
+
+#define min(a,b) (((a) < (b))? (a) : (b))
+
+int foo(int x)
+{
+  return min(x,0);
+}
+
+signed char bar(signed char x)
+{
+  return min(x,0);
+}
+
+/* { dg-final { scan-assembler "cltd" } } */
+/* { dg-final { scan-assembler "sarb" } } */

On Mon, Aug 10, 2020 at 10:36 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> To make amends for the recent (temporary) testsuite failure
> of my new gcc.target/i386/minmax-9.c when compiled with -m32,
> this patch improves the -m32 code we generate for the examples
> in that test case.
>
> Currently, smax(x,0) generates the very cool implementation:
>
> smax0:  movl    4(%esp), %eax
>         cltd
>         notl    %edx
>         andl    %edx, %eax
>         ret
>
> But mysteriously, the related smin(x,0) uses a longer conditional
> move sequence:
>
> smin0:  movl    4(%esp), %eax
>         xorl    %edx, %edx
>         testl   %eax, %eax
>         setg    %dl
>         subl    $1, %edx
>         andl    %edx, %eax
>         ret
>
> With this patch, we now generate the much nicer:
>
> smin0:  movl    4(%esp), %eax
>         cltd
>         andl    %edx, %eax
>         ret
>
> The trick is to expand smin(x,0) as "x < 0 ? x : 0" instead
> of the current "x <= 0 ? x : 0", as the former can take
> advantage of sign_bit_mask operations.
>
> The benefits can also be seen in QImode, where previously:
> smin0:  movl    4(%esp), %edx
>         testb   %dl, %dl
>         setg    %al
>         subl    $1, %eax
>         andl    %edx, %eax
>         ret
>
> now becomes:
>
> smin0:  movl    4(%esp), %edx
>         movl    %edx, %eax
>         sarb    $7, %al
>         andl    %edx, %eax
>         ret
>
>
> This patch has been tested on x86_64-pc-linux-gnu with a
> "make bootstrap" and "make -k check" with no new failures.
> Please let me know if I need to tweak the dejagnu directives;
> no other i386.exp test appears to explicitly specify -m32, but as
> a compile test, it's not unreasonable to check things on x86_64.

We don't use -m32 compile directives in the testsuite, please just use
"dg-do compile { target ia32 } }" dg directive.

BTW: Multilibs can be tested with:

make -j X check RUNTESTFLAGS="--target-board=unix\{,-m32}"

> OK for mainline?

OK with the above testcase fix.

Thanks,
Uros.

>
>
> 2020-08-10  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         * config/i386/i386-expand.c (ix86_expand_int_movcc): Expand
>         signed MIN_EXPR against zero as "x < 0 ? x : 0" instead of
>         "x <= 0 ? x : 0" to enable sign_bit_compare_p optimizations.
>
> gcc/testsuite/ChangeLog
>         * gcc.target/i386/minmax-12.c: New test.
>
> Thanks in advance,
> Roger
> --
> Roger Sayle
> NextMove Software
> Cambridge, UK
>