[Bug tree-optimization/60280] New: gcc.target/arm/ivopts.c and gcc.target/arm/ivopts-2.c failed caused by preserving loop structure.

[Bug tree-optimization/60280] New: gcc.target/arm/ivopts.c and gcc.target/arm/ivopts-2.c failed caused by preserving loop structure.

[amker.cheng at gmail dot com]

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

            Bug ID: 60280
           Summary: gcc.target/arm/ivopts.c and gcc.target/arm/ivopts-2.c
                    failed caused by preserving loop structure.
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: amker.cheng at gmail dot com

gcc.target/arm/ivopts-2.c is like:

/* { dg-do assemble } */
/* { dg-options "-Os -fdump-tree-ivopts -save-temps" } */

extern void foo2 (short*);

void
tr4 (short array[], int n)
{
  int x;
  if (n > 0)
    for (x = 0; x < n; x++)
      foo2 (&array[x]);
}

/* { dg-final { scan-tree-dump-times "PHI <ivtmp" 1 "ivopts"} } */
/* { dg-final { scan-tree-dump-times "PHI <" 1 "ivopts"} } */
/* { dg-final { object-size text <= 26 { target arm_thumb2 } } } */
/* { dg-final { cleanup-tree-dump "ivopts" } } */
/* { dg-final { cleanup-saved-temps "ivopts" } } */

Build with options: -mthumb -mcpu=cortex-a15 -c
The size of text section is 28, causing regression.
Same story happens to ivopts.c

[Bug tree-optimization/60280] gcc.target/arm/ivopts.c and gcc.target/arm/ivopts-2.c failed caused by preserving loop structure.

[amker.cheng at gmail dot com]

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

--- Comment #1 from bin.cheng <amker.cheng at gmail dot com> ---
It's caused by patch at (revision r198333):
http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01530.html

After patching, forwarder basic block 6 in below dump didn't get removed:
tr4 (short int * array, int n)
{
  int x;
  unsigned int x.0;
  unsigned int _7;
  short int * _9;

  <bb 2>:
  if (n_4(D) > 0)
    goto <bb 6>;
  else
    goto <bb 5>;

  <bb 3>:

  <bb 4>:
  # x_14 = PHI <x_11(3), 0(6)>
  x.0_6 = (unsigned int) x_14;
  _7 = x.0_6 * 2;
  _9 = array_8(D) + _7;
  foo2 (_9);
  x_11 = x_14 + 1;
  if (x_11 < n_4(D))
    goto <bb 3>;
  else
    goto <bb 5>;

  <bb 5>:
  return;

  <bb 6>:
  goto <bb 4>;
}

After expanding, pre-header is filled with pre-loop initialization instructions
and the problem turns into a cfglayout problem:
    5: NOTE_INSN_BASIC_BLOCK 2
    2: r115:SI=r0:SI
      REG_DEAD r0:SI
    3: NOTE_INSN_DELETED
    4: NOTE_INSN_FUNCTION_BEG
    7: {cc:CC=cmp(r1:SI,0);r116:SI=r1:SI;}
      REG_DEAD r1:SI
    8: pc={(cc:CC>0)?L24:pc}
      REG_DEAD cc:CC
      REG_BR_PROB 0x1f98
;;  succ:       4
;;              5
   29: L29:
   13: NOTE_INSN_BASIC_BLOCK 3
   14: r0:SI=r110:SI
   15: call [`foo2'] argc:0
      REG_DEAD r0:SI
   16: r110:SI=r110:SI+0x2
   18: cc:CC=cmp(r110:SI,r114:SI)
   19: pc={(cc:CC!=0)?L29:pc}
      REG_DEAD cc:CC
      REG_BR_PROB 0x2333
;;  succ:       3
;;              5
   24: L24:
   25: NOTE_INSN_BASIC_BLOCK 4
   26: r110:SI=r115:SI
      REG_DEAD r115:SI
   27: NOTE_INSN_DELETED
   28: r114:SI=r116:SI*0x2+r110:SI
      REG_DEAD r116:SI
;;  succ:       3
   32: L32:
   33: NOTE_INSN_BASIC_BLOCK 5
;;  succ:       EXIT


After outof_cfglayout, a jump (in bb3) to exit block is introduced:
    5: NOTE_INSN_BASIC_BLOCK 2
    3: NOTE_INSN_DELETED
    4: NOTE_INSN_FUNCTION_BEG
    7: {cc:CC=cmp(r1:SI,0);r1:SI=r1:SI;}
    8: pc={(cc:CC>0)?L24:pc}
      REG_BR_PROB 0x1f98
;;  succ:       6
;;              3
   55: NOTE_INSN_BASIC_BLOCK 3
   56: pc=L32
;;  succ:       7
   29: L29:
   13: NOTE_INSN_BASIC_BLOCK 4
   14: r0:SI=r4:SI
   15: call [`foo2'] argc:0
   16: r4:SI=r4:SI+0x2
   18: cc:CC=cmp(r4:SI,r5:SI)
   19: pc={(cc:CC!=0)?L29:pc}
      REG_BR_PROB 0x2333
;;  succ:       4
;;              5
   58: NOTE_INSN_BASIC_BLOCK 5
   59: pc=L32
;;  succ:       7
   24: L24:
   25: NOTE_INSN_BASIC_BLOCK 6
   26: r4:SI=r0:SI
   27: NOTE_INSN_DELETED
   28: r5:SI=r1:SI*0x2+r4:SI
   61: pc=L29
;;  succ:       4
   32: L32:
   33: NOTE_INSN_BASIC_BLOCK 7
;;  succ:       EXIT

Ideally, basic block reordering could fix this, but before that, pass
pro_and_epilogue threads jump in bb3 to a direct return instruction and bb
reordering can do nothing any more.

So:
1) Unless we can teach passes before pro_and_epilogue to do some bb reordering
work, it's inappropriate to fix it on RTL.
2) It's natural to be fixed on GIMPLE, but it's disruptive because the cfg
stuff are shared by all GIMPLE(even RTL) optimizers. Yet this method makes more
sense than 1).

I am trying to work out a less intrusive patch for stage 4.

[Bug tree-optimization/60280] [4.9 Regression] gcc.target/arm/ivopts.c and gcc.target/arm/ivopts-2.c failed caused by preserving loop structure.

[ramana at gcc dot gnu.org]

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

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-02-26
                 CC|                            |ramana at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #4 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
confirmed.

[Bug tree-optimization/60280] [4.9 Regression] gcc.target/arm/ivopts.c and gcc.target/arm/ivopts-2.c failed caused by preserving loop structure.

[amker at gcc dot gnu.org]

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

--- Comment #5 from amker at gcc dot gnu.org ---
Author: amker
Date: Wed Feb 26 01:49:35 2014
New Revision: 208165

URL: http://gcc.gnu.org/viewcvs?rev=208165&root=gcc&view=rev
Log:

    PR target/60280
    * tree-cfgcleanup.c (tree_forwarder_block_p): Protect loop
    preheaders and latches only if requested.  Fix latch if it
    is removed.
    * tree-ssa-dom.c (tree_ssa_dominator_optimize): Set
    LOOPS_HAVE_PREHEADERS.

    PR target/60280
    * gnat.dg/renaming5.adb: Change to two expected gotos.
    * gcc.dg/tree-ssa/pr21559.c: Change back to three expected
    jump threads.
    * gcc.dg/tree-prof/update-loopch.c: Check two "Invalid sum"
    messages for removed basic block.
    * gcc.dg/tree-ssa/ivopt_1.c: Fix unreliable scanning string.
    * gcc.dg/tree-ssa/ivopt_2.c: Ditto.
    * gcc.dg/tree-ssa/ivopt_3.c: Ditto.
    * gcc.dg/tree-ssa/ivopt_4.c: Ditto.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/tree-prof/update-loopch.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ivopt_1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ivopt_2.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ivopt_3.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ivopt_4.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr21559.c
    trunk/gcc/testsuite/gnat.dg/renaming5.adb
    trunk/gcc/tree-cfgcleanup.c
    trunk/gcc/tree-ssa-dom.c

[Bug tree-optimization/60280] [4.9 Regression] gcc.target/arm/ivopts.c and gcc.target/arm/ivopts-2.c failed caused by preserving loop structure.

[amker.cheng at gmail dot com]

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

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

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

--- Comment #6 from bin.cheng <amker.cheng at gmail dot com> ---
Patch applied.  Fixed I think.

[Bug tree-optimization/60280] [4.9 Regression] gcc.target/arm/ivopts.c and gcc.target/arm/ivopts-2.c failed caused by preserving loop structure.

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Fri Feb 28 13:14:23 2014
New Revision: 208222

URL: http://gcc.gnu.org/viewcvs?rev=208222&root=gcc&view=rev
Log:
2014-02-28  Richard Biener  <rguenther@suse.de>

    PR target/60280
    * tree-cfgcleanup.c (tree_forwarder_block_p): Restrict
    previous fix and only allow to remove trivial pre-headers
    and latches.  Also honor LOOPS_MAY_HAVE_MULTIPLE_LATCHES.
    (remove_forwarder_block): Properly update the latch of
    a loop.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-cfgcleanup.c