[Bug tree-optimization/51491] New: ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[Bug tree-optimization/51491] New: ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[pinskia at gcc dot gnu.org]

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

             Bug #: 51491
           Summary: ccp when converting from alloca should add a CLOBBER
                    to right before __builtin_stack_restore
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: pinskia@gcc.gnu.org


While looking into 51471, I noticed that we don't add a clobber for the
converted alloca which we should have done.

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-12-12
                 CC|                            |tom at codesourcery dot com
     Ever Confirmed|0                           |1
           Severity|normal                      |enhancement

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-12 10:27:10 UTC ---
Confirmed.  Would be appropriate for stage3 - Tom?

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenther at suse dot de,
                   |                            |vries at gcc dot gnu.org

--- Comment #2 from vries at gcc dot gnu.org 2011-12-12 13:12:16 UTC ---
I ran:
...
$ ./install/bin/mips64el-linux-gnu-gcc $src -O3 -g -march=octeon 
-fdump-tree-all -msoft-float
...

for:
- src/gcc-mainline/gcc/testsuite/gcc.c-torture/execute/20040811-1.c
- src/gcc-mainline/gcc/testsuite/gcc.c-torture/execute/vla-dealloc-1.c.

In both cases an alloca_with_align is generated during gimplify from a vla,
which still exists at expand, and ccp leaves the alloca_with_align alone.  So
I'm not sure what I should fix in ccp.

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-12-12 19:51:32 UTC ---
The testcase which I referenced in the bug report is one where it shows adding
a CLOBBER is a good idea.  Anyways the following two functions should produce
the same exact code:
int g(int*);

int f(void)
{
  int tt = 0;
  int t = 4;
  {
    int a[t];
    tt = g(a);
    tt += a[0];
  }
  {
    int a[4];
    tt += g(a);
    tt += a[0];
  }
  return tt;
}
int f1(void)
{
  int tt = 0;
  int t = 4;
  {
    int a[4];
    tt = g(a);
    tt += a[0];
  }
  {
    int a[4];
    tt += g(a);
    tt += a[0];
  }
  return tt;
}
--- CUT ---
Currently without adding the CLOBBER, f's stack size is much bigger than f1's.

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[vries at gcc dot gnu.org]

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

--- Comment #4 from vries at gcc dot gnu.org 2011-12-12 21:25:58 UTC ---
f1.c vs. f.c:
...
int g(int*);            int g(int*);

int f1(void)             |    int f(void)
{                {
  int tt = 0;              int tt = 0;
  int t = 4;              int t = 4;
  {                  {
    int a[4];             |        int a[t];
    tt = g(a);                tt = g(a);
    tt += a[0];                tt += a[0];
  }                  }
  {                  {
    int a[4];                int a[4];
    tt += g(a);                tt += g(a);
    tt += a[0];                tt += a[0];
  }                  }
  return tt;              return tt;
}                }
...

f1.c and f.c are compiled like this (for x86_64):
...
$ gcc -O2 -S f.c f1.c -fdump-tree-all
...

f1.c.149t.optimized has 2 clobbers:
...
f1 ()
{
  int a[4];
  int a[4];
  int tt;
  int D.1712;
  int D.1711;
  int D.1710;

<bb 2>:
  tt_3 = g (&a);
  D.1710_4 = a[0];
  tt_5 = D.1710_4 + tt_3;
  a ={v} {CLOBBER};
  D.1711_6 = g (&a);
  tt_7 = D.1711_6 + tt_5;
  D.1712_8 = a[0];
  tt_9 = D.1712_8 + tt_7;
  a ={v} {CLOBBER};
  return tt_9;

}
...

f.c.149t.optimized has only 1 clobbers:
...
f ()
{
  <unnamed-unsigned:8> D.1726[16];
  int a[4];
  int tt;
  int D.1722;
  int D.1721;
  void * saved_stack.2;
  int D.1719;

<bb 2>:
  saved_stack.2_3 = __builtin_stack_save ();
  tt_18 = g (&D.1726);
  D.1719_19 = MEM[(int[0:D.1713] *)&D.1726][0];
  tt_20 = D.1719_19 + tt_18;
  __builtin_stack_restore (saved_stack.2_3);
  D.1721_21 = g (&a);
  tt_22 = D.1721_21 + tt_20;
  D.1722_23 = a[0];
  tt_24 = D.1722_23 + tt_22;
  a ={v} {CLOBBER};
  return tt_24;

}
...

f1 uses only 16 bytes stack space, since the 2 'a[4]' arrays share the stack
space:
...
grep 'subq.*rsp' f1.s f.s
f1.s:    subq    $16, %rsp
f.s:    subq    $32, %rsp
...

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[vries at gcc dot gnu.org]

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

--- Comment #5 from vries at gcc dot gnu.org 2011-12-12 21:33:03 UTC ---
Created attachment 26061
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26061
tentative patch

Using this tentative patch, now also f in f.c.149t.optimized  has 2 clobbers:
...
f ()
{
  <unnamed-unsigned:8> D.1726[16];
  int a[4];
  int tt;
  int D.1722;
  int D.1721;
  void * saved_stack.2;
  int D.1719;

<bb 2>:
  saved_stack.2_3 = __builtin_stack_save ();
  tt_18 = g (&D.1726);
  D.1719_19 = MEM[(int[0:D.1713] *)&D.1726][0];
  tt_20 = D.1719_19 + tt_18;
  D.1726 ={v} {CLOBBER};
  __builtin_stack_restore (saved_stack.2_3);
  D.1721_21 = g (&a);
  tt_22 = D.1721_21 + tt_20;
  D.1722_23 = a[0];
  tt_24 = D.1722_23 + tt_22;
  a ={v} {CLOBBER};
  return tt_24;

}
...

and f only uses 16 bytes stack space:
...
$ grep 'subq.*rsp' f1.s f.
f1.s:    subq    $16, %rsp
f.s:    subq    $16, %rsp
...

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-12-12 22:15:36 UTC ---
Please use is_gimple_call predicate.

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[vries at gcc dot gnu.org]

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

--- Comment #7 from vries at gcc dot gnu.org 2011-12-13 15:00:35 UTC ---
Created attachment 26070
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26070
update tentative patch

now testing on x86_64

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[vries at gcc dot gnu.org]

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

--- Comment #9 from vries at gcc dot gnu.org 2011-12-17 11:39:53 UTC ---
Author: vries
Date: Sat Dec 17 11:39:49 2011
New Revision: 182433

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182433
Log:
2011-12-17  Tom de Vries  <tom@codesourcery.com>

    PR tree-optimization/51491
    * gcc.dg/pr51491.c: New test.
    * gcc.dg/pr51491-2.c: Same.

Added:
    trunk/gcc/testsuite/gcc.dg/pr51491-2.c
    trunk/gcc/testsuite/gcc.dg/pr51491.c
Modified:
    trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[vries at gcc dot gnu.org]

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

--- Comment #8 from vries at gcc dot gnu.org 2011-12-17 11:39:47 UTC ---
Author: vries
Date: Sat Dec 17 11:39:43 2011
New Revision: 182432

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182432
Log:
2011-12-17  Tom de Vries  <tom@codesourcery.com>

    PR tree-optimization/51491
    * tree-ssa-ccp.c (insert_clobber_before_stack_restore)
    (gsi_prev_dom_bb_nondebug, insert_clobbers_for_var): New function.
    (ccp_fold_stmt): Use insert_clobbers_for_var after a successful
    fold_builtin_alloca_with_align.
    (ccp_visit_stmt): Calculate and free dominator info.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-ccp.c

[Bug tree-optimization/51491] ccp when converting from alloca should add a CLOBBER to right before __builtin_stack_restore

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

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

--- Comment #10 from vries at gcc dot gnu.org 2011-12-17 11:43:06 UTC ---
fix and testcase checked in.

Andrew, thanks for noticing this.