[Bug regression/60363] New: [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[Bug regression/60363] New: [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

            Bug ID: 60363
           Summary: [4.9 Regression]: logical_op_short_circuit,
                    gcc.dg/tree-ssa/ssa-dom-thread-4.c
                    scan-tree-dump-times dom1 "Threaded" 4
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: regression
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hp at gcc dot gnu.org
                CC: amker at gcc dot gnu.org
              Host: x86_64-unknown-linux-gnu
            Target: cris-axis-elf

This test previously passed, now it fails.
A patch in the revision range (last_known_working:first_known_failing)
208159:208165 exposed or caused this regression.  Since then it fails as
follows:

Running /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.dg/tree-ssa/tree-ssa.exp ...
...
FAIL: gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

This is a logical_op_short_circuit target, and the FAIL seems common to such
targets, cf: <http://gcc.gnu.org/ml/gcc-testresults/2014-02/msg01894.html>
(AVR being the only target with test-results properly marked as such).
A target that may be of interest is too; "ARM cortex-M".

Author of suspect patch in revision range CC:ed.
ssa-dom-thread-4.c.078t.dom1 from r208207 attached. (There are 6 "Threaded")

[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #1 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
Created attachment 32230
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32230&action=edit
ssa-dom-thread-4.c.078t.dom1 for cris-elf.

[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.9.0

[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

bin.cheng <amker.cheng at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amker.cheng at gmail dot com

--- Comment #2 from bin.cheng <amker.cheng at gmail dot com> ---
Created attachment 32315
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32315&action=edit
tar of dump files.

[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #3 from bin.cheng <amker.cheng at gmail dot com> ---
After patching 208165, there are two more jump threading opportunities for dom1
pass.  Jump threading is doing alright, the interesting thing is why there is
no such opportunities before patching.

I attatched related dump files with/without patch.  It seems dumps before vrp1
pass are pretty similar, while after vrp1, dump with patch shows the two
additional jump threading opportunities.  In other words, they are somehow
already fixed (not introduced) in pass vrp1 without patching.

For now I can just change ssa-dom-thread-4.c to handle the two jump threadings,
or should I look into vrp to find the difference in the first place?

[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #4 from bin.cheng <amker.cheng at gmail dot com> ---
Although may be irrelavant.  I found loop's latch doesn't get updated after
removing the forwarder latch basic block.  Previous patch only catches function
remove_forwarder_block, but remove_forwarder_block_with_phi should be handled
too.

I will send a patch picking this up.

[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #5 from bin.cheng <amker.cheng at gmail dot com> ---
Vrp1 generates below code:


  <bb 3>:
  if (b_elt_11(D) != 0B)
    goto <bb 4>;
  else
    goto <bb 8>;

  <bb 4>:
  # kill_elt_10 = PHI <kill_elt_4(3)>
  goto <bb 6>;

  <bb 5>:
  kill_elt_14 = kill_elt_2->next;

  <bb 6>:
  # kill_elt_2 = PHI <kill_elt_10(4), kill_elt_14(5)>
  if (kill_elt_2 != 0B)
    goto <bb 7>;
  else
    goto <bb 19>;

  <bb 7>:
  _12 = kill_elt_2->indx;
  _13 = b_elt_11(D)->indx;
  if (_12 < _13)
    goto <bb 5>;
  else
    goto <bb 20>;

...


  <bb 18>:
  goto <bb 14>;

  <bb 19>:
  # kill_elt_41 = PHI <0B(6)>
  if (b_elt_11(D) != 0B)
    goto <bb 18>;
  else
    goto <bb 14>;

The whole bb 19 is unnecessary since we know "b_elt_11(D) != 0" holds.

[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #6 from bin.cheng <amker.cheng at gmail dot com> ---
After investigation, I think the root cause is:

For given mergephi2 dump (before vrp), there are latch(bb13) and header(bb14)
of a loop:

  <bb 13>:
  # changed_18 = PHI <changed_26(12), changed_1(7), changed_1(8), changed_1(9),
changed_1(11)>

  <bb 14>:
  # changed_1 = PHI <changed_18(13), 0(2)>
  # kill_elt_4 = PHI <kill_elt_3(13), kill_elt_7(D)(2)>
  if (a_elt_9(D) != 0B)
    goto <bb 3>;
  else
    goto <bb 15>;

The latch is removed by the bogus patch, and the code turns into:

  <bb 14>:
  # changed_1 = PHI <changed_1(7), 0(2), changed_26(12), changed_1(11),
changed_1(9), changed_1(8)>
  # kill_elt_4 = PHI <kill_elt_3(7), kill_elt_7(D)(2), kill_elt_3(12),
kill_elt_3(11), kill_elt_3(9), kill_elt_3(8)>
  if (a_elt_9(D) != 0B)
    goto <bb 3>;
  else
    goto <bb 15>;

Since VRP requires LOOP_HAVE_SIMPLE_LATCH, it will be initialized into:

  <bb 14>:
  # changed_19 = PHI <changed_1(7), changed_1(22), changed_26(12),
changed_1(11), changed_1(9)>
  # kill_elt_18 = PHI <kill_elt_3(7), kill_elt_33(22), kill_elt_32(12),
kill_elt_32(11), kill_elt_32(9)>

  <bb 17>:
  # changed_1 = PHI <changed_19(14), 0(2)>
  # kill_elt_4 = PHI <kill_elt_18(14), kill_elt_7(D)(2)>
  if (a_elt_9(D) != 0B)
    goto <bb 24>;
  else
    goto <bb 15>;

After all works in vrp (including jump threading), the code is transformed into
below form:


  <bb 18>:
  # kill_elt_2 = PHI <kill_elt_10(5), kill_elt_14(4)>
  if (kill_elt_2 != 0B)
    goto <bb 6>;
  else
    goto <bb 21>;

  <bb 21>:
  goto <bb 27>;

...

  <bb 14>:
  # changed_19 = PHI <changed_1(7), changed_1(22), changed_26(12),
changed_1(11), changed_1(9), changed_1(27), changed_1(29)>
  # kill_elt_18 = PHI <kill_elt_3(7), 0B(26), kill_elt_3(12), kill_elt_3(11),
kill_elt_3(9), kill_elt_3(27), kill_elt_3(29)>

  <bb 17>:
  # changed_1 = PHI <changed_19(14), 0(2)>
  # kill_elt_4 = PHI <kill_elt_18(14), kill_elt_7(D)(2)>
  if (a_elt_9(D) != 0B)
    goto <bb 24>;
  else
    goto <bb 15>;

...

  <bb 26>:
  goto <bb 14>;

  <bb 27>:
  # kill_elt_41 = PHI <0B(21)>
  if (b_elt_11(D) != 0B)
    goto <bb 26>;
  else
    goto <bb 14>;

When trying to remove bb26, it finds out bb27 is pred of both bb26 and bb14,
then checks whether the phi args are consistent for both edges (<26, 14> and
<27, 14>).  Apparently "0B(26)" != "kill_elt_3(27)", resulting in keeping bb27.

Actually, kill_elt_3(27) equals to "0", because of check condition in bb18.

So this might be a missed opportunity in vrp?

[Bug regression/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #7 from bin.cheng <amker.cheng at gmail dot com> ---
The problem has nothing to do with VRP, and might be a missed opportunity of
jump threading.

The dump after VRP but before jump threading at the end of VRP is like:

...

  <bb 21>:
  goto <bb 7>;

...

  <bb 7>:
  # kill_elt_3 = PHI <kill_elt_4(20), 0B(21), kill_elt_2(6)>
  if (b_elt_11(D) != 0B)
    goto <bb 8>;
  else
    goto <bb 14>;

  <bb 8>:
  if (kill_elt_3 != 0B)
    goto <bb 9>;
  else
    goto <bb 22>;

  <bb 22>:
  goto <bb 14>;

...

  <bb 14>:
  # changed_19 = PHI <changed_1(7), changed_1(22), changed_26(12),
changed_1(11), changed_1(9)>
  # kill_elt_18 = PHI <kill_elt_3(7), 0B(22), kill_elt_3(12), kill_elt_3(11),
kill_elt_3(9)>

  <bb 17>:
  # changed_1 = PHI <changed_19(14), 0(2)>
  # kill_elt_4 = PHI <kill_elt_18(14), kill_elt_7(D)(2)>
  if (a_elt_9(D) != 0B)
    goto <bb 24>;
  else
    goto <bb 15>;

There is jump threading path like:
  Registering jump thread: (21, 7) incoming edge;  (7, 8) joiner;  (8, 22)
normal;

After jump threading, the dump is like:
  <bb 18>:
  # kill_elt_2 = PHI <kill_elt_10(5), kill_elt_14(4)>
  if (kill_elt_2 != 0B)
    goto <bb 6>;
  else
    goto <bb 21>;

  <bb 21>:
  goto <bb 27>;

...

  <bb 14>:
  # changed_19 = PHI <changed_1(7), changed_1(22), changed_26(12),
changed_1(11), changed_1(9), changed_1(27), changed_1(29)>
  # kill_elt_18 = PHI <kill_elt_3(7), 0B(26), kill_elt_3(12), kill_elt_3(11),
kill_elt_3(9), kill_elt_3(27), kill_elt_3(29)>

  <bb 17>:
  # changed_1 = PHI <changed_19(14), 0(2)>
  # kill_elt_4 = PHI <kill_elt_18(14), kill_elt_7(D)(2)>
  if (a_elt_9(D) != 0B)
    goto <bb 24>;
  else
    goto <bb 15>;

...

  <bb 26>:
  goto <bb 14>;

  <bb 27>:
  # kill_elt_41 = PHI <0B(21)>
  if (b_elt_11(D) != 0B)
    goto <bb 26>;
  else
    goto <bb 14>;

During jump threading process, GCC updates phi node in destination basic block
like "kill_elt_18 = PHI <".  Because bb27 is duplicated from bb7, the phi
argument "kill_elt_3(27)" is copied directly from "kill_elt_3(7)".  Although
"kill_elt_3 == 0" holds when coming from edge <bb27, bb14>, gcc doesn't know
this fact.
At last, cfgcleanup tries to remove forwarder block bb26, but it can't make it
because the phi arguments from edge<bb27, bb14> and edge<bb26, bb14> doesn't
match.

I am thinking maybe jump threading can back trace the threading path and try to
add phi argument for edge<bb27, bb14> with the value(0) of kill_elt_3 (i.e.,
"0(27)").  The value is available along the jump threading path.

[Bug tree-optimization/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|cris-axis-elf               |cris-axis-elf,
                   |                            |mipsel-unknown-linux-gnu,
                   |                            |mips-unknown-linux-gnu
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-03-31
          Component|regression                  |tree-optimization
            Version|4.7.0                       |4.9.0
     Ever confirmed|0                           |1

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fails on mips[el]-unknown-linux-gnu (and thus likely mipsisa64-elf, a primary
target) as well.

Consider fixing it by xfail-ing for logical_op_short_circuit targets if a fix
isn't suitable at this stage.

[Bug tree-optimization/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #10 from bin.cheng <amker.cheng at gmail dot com> ---
Patch sent at http://gcc.gnu.org/ml/gcc-patches/2014-03/msg00857.html , but it
need to wait for stage 1.

I will xfail it for now.

[Bug tree-optimization/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #11 from Jeffrey A. Law <law at redhat dot com> ---
Agreed, let's xfail the test on the affected targets for now and look to fix
the missed threading optimization during the next stage1.

[Bug tree-optimization/60363] [4.9 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |xfail
           Priority|P1                          |P3

[Bug tree-optimization/60363] [4.9/4.10 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.9.0                       |4.9.1

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.9.0 has been released

[Bug tree-optimization/60363] [4.9/4.10 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

--- Comment #15 from bin.cheng <amker.cheng at gmail dot com> ---
Should be fixed now.

[Bug tree-optimization/60363] [4.9/4.10 Regression]: logical_op_short_circuit, gcc.dg/tree-ssa/ssa-dom-thread-4.c scan-tree-dump-times dom1 "Threaded" 4

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
      Known to work|                            |4.10.0
         Resolution|---                         |FIXED
   Target Milestone|4.9.1                       |4.10.0
      Known to fail|                            |4.9.1

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