[Bug tree-optimization/102008] New: [12 Regression] no cmov generated for loads next to each other

[Bug tree-optimization/102008] New: [12 Regression] no cmov generated for loads next to each other

[pinskia at gcc dot gnu.org]

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

            Bug ID: 102008
           Summary: [12 Regression] no cmov generated for loads next to
                    each other
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
  Target Milestone: ---
            Target: x86_64-*-*

Take:
struct Foo {  int a;  int b; };

int test(int side, const Foo *foo) {
  if (side == 1) return foo->a;
  return foo->b;
}
----- CUT ----
Before r12-897, GCC was able to produce a cmov for this case but now we don't.
Note for aarch64 we produce now:
        cmp     w0, 1
        add     x0, x1, 4
        csel    x0, x0, x1, ne
        ldr     w0, [x0]
Which is actually reasonible still.

Note I noticed this while looking into PR 68274.

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
          Component|tree-optimization           |rtl-optimization

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-08-23
     Ever confirmed|0                           |1
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |NEW
                 CC|                            |luoxhu at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Oh, the late sinking pass now undoes PHI-OPTs hoist_adjacent_loads - that's
ping-ponging already between phiopt2, sink1 and phiopt4.  So the easiest
"fix" would be to re-schedule the added sink pass before phiopt4.

Of course having two transforms that work against each other in the compiler is
bad in the first place.

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[luoxhu at gcc dot gnu.org]

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

--- Comment #2 from luoxhu at gcc dot gnu.org ---
Confirmed if move the sink2 pass before phiopt4 could restore the previous
instructons for this case:

test:
.LFB0:
        .cfi_startproc
        cmp     w0, 1
        ldp     w0, w1, [x1]
        csel    w0, w1, w0, ne
        ret
        .cfi_endproc



diff --git a/gcc/passes.def b/gcc/passes.def
index 945d2bc797c..83b8310f1ee 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -345,10 +345,10 @@ along with GCC; see the file COPYING3.  If not see
       /* After late CD DCE we rewrite no longer addressed locals into SSA
         form if possible.  */
       NEXT_PASS (pass_forwprop);
+      NEXT_PASS (pass_sink_code);
       NEXT_PASS (pass_phiopt, false /* early_p */);
       NEXT_PASS (pass_fold_builtins);
       NEXT_PASS (pass_optimize_widening_mul);
-      NEXT_PASS (pass_sink_code);
       NEXT_PASS (pass_store_merging);
       NEXT_PASS (pass_tail_calls);


ls *sink*
pr102008.c.139t.sink1  pr102008.c.199t.sink2
ls *phiopt*
pr102008.c.042t.phiopt1  pr102008.c.119t.phiopt2  pr102008.c.131t.phiopt3 
pr102008.c.200t.phiopt4

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[luoxhu at gcc dot gnu.org]

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

--- Comment #3 from luoxhu at gcc dot gnu.org ---

phiopt4 and sink2 are doing reverse optimizations:

pr102008.c.200t.phiopt4: 

 Hoisting adjacent loads from 3 and 4 into 2:  _6 = foo_4(D)->a;  _5 =
foo_4(D)->b;

pr102008.c.202t.sink2: 

 Sinking _5 = foo_4(D)->b; from bb 2 to bb 4
 Sinking  _6 = foo_4(D)->a; from bb 2 to bb 3

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
I'm going to test the re-scheduling now.

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Wouldn't at least now simply disabling the "optimization" in the last sink pass
instance be safer?

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> Wouldn't at least now simply disabling the "optimization" in the last sink
> pass instance be safer?

I don't see how that's easily done.  Not sinking any loads would be as
intrusive as doing pass-reordering, adding code to detect exactly the
"two adjacent loads split into two places" would be quite elaborate.

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ok.  Though, perhaps indeed trying to detect what phiopt optimizes and not
sinking that case would be long term best, so that we don't do the ping pong.

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #7)
> Ok.  Though, perhaps indeed trying to detect what phiopt optimizes and not
> sinking that case would be long term best, so that we don't do the ping pong.

Or, since the phiopt case is specifically for if-conversion (IIRC), only do
this in the very last phiopt pass before RTL expansion and/or perform the
if-conversion at the GIMPLE level via a COND_EXPR, thus actually do the
if-conversion.

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:f6fb661ea8ac7e17c6924719de6219f002c4efef

commit r12-7670-gf6fb661ea8ac7e17c6924719de6219f002c4efef
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Mar 16 13:39:31 2022 +0100

    tree-optimization/102008 - restore if-conversion of adjacent loads

    The following re-orders the newly added code sinking pass before
    the last phiopt pass which performs hoisting of adjacent loads
    with the intent to enable if-conversion on those.

    I've added the aarch64 specific testcase from the PR.

    2022-03-16  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/102008
            * passes.def: Move the added code sinking pass before the
            preceeding phiopt pass.

            * gcc.target/aarch64/pr102008.c: New testcase.

[Bug rtl-optimization/102008] [12 Regression] no cmov generated for loads next to each other

[rguenth at gcc dot gnu.org]

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

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

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

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