[PATCH, i386]: Fix PR 65871, bzhi builtin/intrinsic wrongly assumes bzhi instruction doesn't set the ZF flag

[PATCH, i386]: Fix PR 65871, bzhi builtin/intrinsic wrongly assumes bzhi instruction doesn't set the ZF flag

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

Hello!

Attached patch implements a CCZ-only setting pattern for a couple of
BMI[,2] intrinsics.

2015-04-29  Uros Bizjak  <ubizjak@gmail.com>

    PR target/65871
    * config/i386/i386.md (*bmi_bextr_<mode>_cczonly): New pattern.
    (*bmi2_bzhi_<mode>3_1_cczonly): Ditto.

testsuite/ChangeLog:

2015-04-29  Uros Bizjak  <ubizjak@gmail.com>

    PR target/65871
    * gcc.target/i386/pr65871-1.c: New test
    * gcc.target/i386/pr65871-2.c: Ditto.

Patch was bootstrapped and regression tested on x86_64-linux-gnu
{,-m32} and was committed to mainline SVN.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 2887 bytes --]

Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 222585)
+++ config/i386/i386.md	(working copy)
@@ -12594,6 +12594,20 @@
    (set_attr "btver2_decode" "direct, double")
    (set_attr "mode" "<MODE>")])
 
+(define_insn "*bmi_bextr_<mode>_cczonly"
+  [(set (reg:CCZ FLAGS_REG)
+	(compare:CCZ
+	  (unspec:SWI48 [(match_operand:SWI48 1 "nonimmediate_operand" "r,m")
+			 (match_operand:SWI48 2 "register_operand" "r,r")]
+			UNSPEC_BEXTR)
+	  (const_int 0)))
+   (clobber (match_scratch:SWI48 0 "=r,r"))]
+  "TARGET_BMI"
+  "bextr\t{%2, %1, %0|%0, %1, %2}"
+  [(set_attr "type" "bitmanip")
+   (set_attr "btver2_decode" "direct, double")
+   (set_attr "mode" "<MODE>")])
+
 (define_insn "*bmi_blsi_<mode>"
   [(set (match_operand:SWI48 0 "register_operand" "=r")
         (and:SWI48
@@ -12667,6 +12681,7 @@
    (set_attr "mode" "<MODE>")])
 
 (define_mode_attr k [(SI "k") (DI "q")])
+
 (define_insn "*bmi2_bzhi_<mode>3_1"
   [(set (match_operand:SWI48 0 "register_operand" "=r")
 	(zero_extract:SWI48
@@ -12682,6 +12697,23 @@
    (set_attr "prefix" "vex")
    (set_attr "mode" "<MODE>")])
 
+(define_insn "*bmi2_bzhi_<mode>3_1_cczonly"
+  [(set (reg:CCZ FLAGS_REG)
+	(compare:CCZ
+	  (zero_extract:SWI48
+	    (match_operand:SWI48 1 "nonimmediate_operand" "rm")
+	    (umin:SWI48
+	      (zero_extend:SWI48 (match_operand:QI 2 "register_operand" "r"))
+	      (match_operand:SWI48 3 "const_int_operand" "n"))
+	    (const_int 0))
+	(const_int 0)))
+   (clobber (match_scratch:SWI48 0 "=r"))]
+  "TARGET_BMI2 && INTVAL (operands[3]) == <MODE_SIZE> * BITS_PER_UNIT"
+  "bzhi\t{%<k>2, %1, %0|%0, %1, %<k>2}"
+  [(set_attr "type" "bitmanip")
+   (set_attr "prefix" "vex")
+   (set_attr "mode" "<MODE>")])
+
 (define_insn "bmi2_pdep_<mode>3"
   [(set (match_operand:SWI48 0 "register_operand" "=r")
         (unspec:SWI48 [(match_operand:SWI48 1 "register_operand" "r")
Index: testsuite/gcc.target/i386/pr65871-1.c
===================================================================
--- testsuite/gcc.target/i386/pr65871-1.c	(revision 0)
+++ testsuite/gcc.target/i386/pr65871-1.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mbmi" } */
+
+int foo (unsigned int x, unsigned int y)
+{
+  if (__builtin_ia32_bextr_u32 (x, y))
+    return 1;
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not "test" } } */
Index: testsuite/gcc.target/i386/pr65871-2.c
===================================================================
--- testsuite/gcc.target/i386/pr65871-2.c	(revision 0)
+++ testsuite/gcc.target/i386/pr65871-2.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mbmi2" } */
+
+int foo (unsigned int x, unsigned int y)
+{
+  if (__builtin_ia32_bzhi_si (x, y))
+    return 1;
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not "test" } } */

Re: [PATCH, i386]: Fix PR 65871, bzhi builtin/intrinsic wrongly assumes bzhi instruction doesn't set the ZF flag

[Jakub Jelinek]

On Wed, Apr 29, 2015 at 09:02:16PM +0200, Uros Bizjak wrote:
> Hello!
> 
> Attached patch implements a CCZ-only setting pattern for a couple of
> BMI[,2] intrinsics.

What about the case when the result is used, but also tested for zero or
non-zero?  Like e.g.
int bar (void);
int foo (unsigned int x, unsigned int y)
{
  int v = __builtin_ia32_bextr_u32 (x, y);
  if (v)
    return v;
  return bar ();
}

	Jakub

Re: [PATCH, i386]: Fix PR 65871, bzhi builtin/intrinsic wrongly assumes bzhi instruction doesn't set the ZF flag

[Uros Bizjak]

On Wed, Apr 29, 2015 at 9:23 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Apr 29, 2015 at 09:02:16PM +0200, Uros Bizjak wrote:
>> Hello!
>>
>> Attached patch implements a CCZ-only setting pattern for a couple of
>> BMI[,2] intrinsics.
>
> What about the case when the result is used, but also tested for zero or
> non-zero?  Like e.g.
> int bar (void);
> int foo (unsigned int x, unsigned int y)
> {
>   int v = __builtin_ia32_bextr_u32 (x, y);
>   if (v)
>     return v;
>   return bar ();
> }

Yes, I have considered this usage, but it would require *another*
pattern. I don't think it is worth just to have one test insn less...

Uros.

Re: [PATCH, i386]: Fix PR 65871, bzhi builtin/intrinsic wrongly assumes bzhi instruction doesn't set the ZF flag

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On Wed, Apr 29, 2015 at 9:02 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
> Attached patch implements a CCZ-only setting pattern for a couple of
> BMI[,2] intrinsics.

These two insns are three operand instructions, and some peephole2
patterns were not prepared for this. Please note that clobbered
register can be different than input registers. Attached patch fixes
this omission.

2015-04-29  Uros Bizjak  <ubizjak@gmail.com>

    PR target/65871
    * config/i386/i386.md (*bmi_bextr_<mode>_cczonly): New pattern.
    (*bmi2_bzhi_<mode>3_1_cczonly): Ditto.
    (setcc+movzbl peephole2): Check also clobbered reg.
    (setcc+andl peephole2): Ditto.

Tested on x86_64-linux-gnu {,-m32} and committed to mainline SVN.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 1140 bytes --]

Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 222590)
+++ config/i386/i386.md	(working copy)
@@ -11567,7 +11567,9 @@
 	(zero_extend (match_dup 1)))]
   "(peep2_reg_dead_p (3, operands[1])
     || operands_match_p (operands[1], operands[3]))
-   && ! reg_overlap_mentioned_p (operands[3], operands[0])"
+   && ! reg_overlap_mentioned_p (operands[3], operands[0])
+   && ! (GET_CODE (operands[4]) == CLOBBER
+	 && reg_mentioned_p (operands[3], operands[4]))"
   [(parallel [(set (match_dup 5) (match_dup 0))
 	      (match_dup 4)])
    (set (strict_low_part (match_dup 6))
@@ -11610,7 +11612,9 @@
 	      (clobber (reg:CC FLAGS_REG))])]
   "(peep2_reg_dead_p (3, operands[1])
     || operands_match_p (operands[1], operands[3]))
-   && ! reg_overlap_mentioned_p (operands[3], operands[0])"
+   && ! reg_overlap_mentioned_p (operands[3], operands[0])
+   && ! (GET_CODE (operands[4]) == CLOBBER
+	 && reg_mentioned_p (operands[3], operands[4]))"
   [(parallel [(set (match_dup 5) (match_dup 0))
 	      (match_dup 4)])
    (set (strict_low_part (match_dup 6))