[Bug target/104982] New: [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[Bug target/104982] New: [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

            Bug ID: 104982
           Summary: [12 Regression] FAIL: gcc.target/i386/bt-5.c by
                    r12-7687
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hjl.tools at gmail dot com
                CC: crazylht at gmail dot com, rguenth at gcc dot gnu.org,
                    roger at nextmovesoftware dot com
            Blocks: 46235
  Target Milestone: ---

On x86-64, r12-7687 caused:

FAIL: gcc.target/i386/bt-5.c scan-assembler-not sar[lq][ \t]
FAIL: gcc.target/i386/bt-5.c scan-assembler-times bt[lq][ \t] 7


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46235
[Bug 46235] inefficient bittest code generation

[Bug target/104982] [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64-*-* i?86-*-*
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-03-21
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |missed-optimization
   Target Milestone|---                         |12.0

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  From my analysis at the point I pushed the offending revision the
edges out of a conditional are now swapped, EDGE_SUCC (bb, 1) vs. EDGE_SUCC
(bb, 0), and that somehow leads to magic no longer happening.

That's of course a condition (the particular edge order) that we can't rely
on.  If there's a "better" one for expanding RTL we should conciously choose
it there.

[Bug target/104982] [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

--- Comment #2 from Hongtao.liu <crazylht at gmail dot com> ---
334Failed to match this instruction:
335(set (reg/v:SI 88 [ z ])
336    (if_then_else:SI (eq (zero_extract:SI (reg:SI 92)
337                (const_int 1 [0x1])
338                (zero_extend:SI (subreg:QI (reg:SI 93) 0)))
339            (const_int 0 [0]))
340        (reg:SI 95)
341        (reg:SI 94)))
342Failed to match this instruction:
343(set (reg/v:SI 88 [ z ])
344    (if_then_else:SI (eq (zero_extract:SI (reg:SI 92)
345                (const_int 1 [0x1])
346                (and:SI (reg:SI 93)
347                    (const_int 255 [0xff])))
348            (const_int 0 [0]))
349        (reg:SI 95)
350        (reg:SI 94)))

And original splitter is

+;; Help combine recognize bt followed by cmov
+(define_split
+  [(set (match_operand:SWI248 0 "register_operand")
+       (if_then_else:SWI248
+        (ne
+         (zero_extract:SWI48
+          (match_operand:SWI48 1 "register_operand")
+          (const_int 1)
+          (zero_extend:SI (match_operand:QI 2 "register_operand")))
+         (const_int 0))
+        (match_operand:SWI248 3 "nonimmediate_operand")
+        (match_operand:SWI248 4 "nonimmediate_operand")))]
+  "TARGET_USE_BT && TARGET_CMOVE
+   && !(MEM_P (operands[3]) && MEM_P (operands[4]))
+   && ix86_pre_reload_split ()"
+  [(set (reg:CCC FLAGS_REG)
+       (compare:CCC
+        (zero_extract:SWI48 (match_dup 1) (const_int 1) (match_dup 2))
+        (const_int 0)))
+   (set (match_dup 0)
+       (if_then_else:SWI248 (eq (reg:CCC FLAGS_REG) (const_int 0))
+                            (match_dup 3)
+                            (match_dup 4)))]
+{
+  operands[2] = lowpart_subreg (SImode, operands[2], QImode);
+})

[Bug target/104982] [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

--- Comment #3 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Hongtao.liu from comment #2)
> 334Failed to match this instruction:
> 335(set (reg/v:SI 88 [ z ])
> 336    (if_then_else:SI (eq (zero_extract:SI (reg:SI 92)
> 337                (const_int 1 [0x1])
> 338                (zero_extend:SI (subreg:QI (reg:SI 93) 0)))
> 339            (const_int 0 [0]))
> 340        (reg:SI 95)
> 341        (reg:SI 94)))
And it's equal to

> 335(set (reg/v:SI 88 [ z ])
> 336    (if_then_else:SI (ne (zero_extract:SI (reg:SI 92)
> 337                (const_int 1 [0x1])
> 338                (zero_extend:SI (subreg:QI (reg:SI 93) 0)))
> 339            (const_int 0 [0]))
> 340        (reg:SI 94)
> 341        (reg:SI 95)))

by swapping 'then' and 'else' rtx.

Guess generic part won't give a second try for such reversed condition?

[Bug target/104982] [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

--- Comment #4 from Hongtao.liu <crazylht at gmail dot com> ---
I'm testing

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 02f298c2846..c74edd1aaef 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -14182,12 +14182,12 @@ (define_insn_and_split "*jcc_bt<mode>_mask"
 (define_split
   [(set (match_operand:SWI248 0 "register_operand")
        (if_then_else:SWI248
-        (ne
-         (zero_extract:SWI48
-          (match_operand:SWI48 1 "register_operand")
-          (const_int 1)
-          (zero_extend:SI (match_operand:QI 2 "register_operand")))
-         (const_int 0))
+        (match_operator 5 "bt_comparison_operator"
+         [(zero_extract:SWI48
+           (match_operand:SWI48 1 "register_operand")
+           (const_int 1)
+           (zero_extend:SI (match_operand:QI 2 "register_operand")))
+          (const_int 0)])
         (match_operand:SWI248 3 "nonimmediate_operand")
         (match_operand:SWI248 4 "nonimmediate_operand")))]
   "TARGET_USE_BT && TARGET_CMOVE
@@ -14202,6 +14202,8 @@ (define_split
                             (match_dup 3)
                             (match_dup 4)))]
 {
+  if (GET_CODE (operands[5]) == EQ)
+    std::swap (operands[3], operands[4]);
   operands[2] = lowpart_subreg (SImode, operands[2], QImode);
 })

[Bug target/104982] [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Hongtao.liu from comment #4)
> I'm testing
> 
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 02f298c2846..c74edd1aaef 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -14182,12 +14182,12 @@ (define_insn_and_split "*jcc_bt<mode>_mask"
>  (define_split
>    [(set (match_operand:SWI248 0 "register_operand")
>         (if_then_else:SWI248
> -        (ne
> -         (zero_extract:SWI48
> -          (match_operand:SWI48 1 "register_operand")
> -          (const_int 1)
> -          (zero_extend:SI (match_operand:QI 2 "register_operand")))
> -         (const_int 0))
> +        (match_operator 5 "bt_comparison_operator"
> +         [(zero_extract:SWI48
> +           (match_operand:SWI48 1 "register_operand")
> +           (const_int 1)
> +           (zero_extend:SI (match_operand:QI 2 "register_operand")))
> +          (const_int 0)])
>          (match_operand:SWI248 3 "nonimmediate_operand")
>          (match_operand:SWI248 4 "nonimmediate_operand")))]
>    "TARGET_USE_BT && TARGET_CMOVE
> @@ -14202,6 +14202,8 @@ (define_split
>                              (match_dup 3)
>                              (match_dup 4)))]
>  {
> +  if (GET_CODE (operands[5]) == EQ)
> +    std::swap (operands[3], operands[4]);
>    operands[2] = lowpart_subreg (SImode, operands[2], QImode);
>  })

Ah, yeah - I was hoping for a solution like this.

[Bug target/104982] [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:919fbffef0755562cd3b686c838069c20bc7878f

commit r12-7751-g919fbffef0755562cd3b686c838069c20bc7878f
Author: liuhongt <hongtao.liu@intel.com>
Date:   Mon Mar 21 20:54:30 2022 +0800

    Extend splitter pattern to reversed condition by swapping then and else
rtx. [PR target/104982]

    Failed to match this instruction:
    (set (reg/v:SI 88 [ z ])
        (if_then_else:SI (eq (zero_extract:SI (reg:SI 92)
                    (const_int 1 [0x1])
                    (zero_extend:SI (subreg:QI (reg:SI 93) 0)))
                (const_int 0 [0]))
            (reg:SI 95)
            (reg:SI 94)))

    but it's equal to

    (set (reg/v:SI 88 [ z ])
        (if_then_else:SI (ne (zero_extract:SI (reg:SI 92)
                    (const_int 1 [0x1])
                    (zero_extend:SI (subreg:QI (reg:SI 93) 0)))
                (const_int 0 [0]))
            (reg:SI 94)
            (reg:SI 95)))

    which is the exact existing splitter.

    The patch will fix below regressions:

    On x86-64, r12-7687 caused:

    FAIL: gcc.target/i386/bt-5.c scan-assembler-not sar[lq][ \t]
    FAIL: gcc.target/i386/bt-5.c scan-assembler-times bt[lq][ \t] 7

    gcc/ChangeLog:

            PR target/104982
            * config/i386/i386.md (*jcc_bt<mode>_mask): Extend the
            following splitter to reversed condition.

[Bug target/104982] [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

--- Comment #7 from Hongtao.liu <crazylht at gmail dot com> ---
Fixed in GCC12.

[Bug target/104982] [12 Regression] FAIL: gcc.target/i386/bt-5.c by r12-7687

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104982

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
.