[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener  ---
Author: rguenth
Date: Tue Mar 21 11:43:45 2017
New Revision: 246314

URL: https://gcc.gnu.org/viewcvs?rev=246314=gcc=rev
Log:
2017-03-21  Richard Biener  

PR tree-optimization/80032
* gimplify.c (gimple_push_cleanup): Add force_uncond parameter,
if set force the cleanup to happen unconditionally.
(gimplify_target_expr): Push inserted clobbers with force_uncond
to avoid them being removed by control-dependent DCE.

* g++.dg/opt/pr80032.C: New testcase.

Added:
trunk/gcc/testsuite/g++.dg/opt/pr80032.C
Modified:
trunk/gcc/ChangeLog
trunk/gcc/gimplify.c
trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[jan.smets at nokia dot com]

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

--- Comment #9 from Jan Smets  ---
The alternative patch to gimplify.c seems to run fine.
Thanks

[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener  ---
Optimization regressions:

FAIL: g++.dg/pr64191.C  -std=gnu++11  scan-tree-dump-times cddce1 "CLOBBER" 1
FAIL: g++.dg/pr64191.C  -std=gnu++11  scan-tree-dump-times cddce1 "if" 0
FAIL: g++.dg/pr64191.C  -std=gnu++14  scan-tree-dump-times cddce1 "CLOBBER" 1
FAIL: g++.dg/pr64191.C  -std=gnu++14  scan-tree-dump-times cddce1 "if" 0
FAIL: g++.dg/pr64191.C  -std=gnu++98  scan-tree-dump-times cddce1 "CLOBBER" 1
FAIL: g++.dg/pr64191.C  -std=gnu++98  scan-tree-dump-times cddce1 "if" 0

it's a real regression in that the loop persists until the end of the
compilation.

I am now fully testing the alternative gimplification.

[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener  ---
The patch causes some optimization regressions.

A different approach is to put the clobbers in non-conditional context -
something that is valid but with the clobber semantic cannot be done by the
middle-end.  Instead we can do this at gimplification time with, say,

Index: gcc/gimplify.c
===
--- gcc/gimplify.c  (revision 246216)
+++ gcc/gimplify.c  (working copy)
@@ -6288,10 +6288,13 @@ gimplify_cleanup_point_expr (tree *expr_

 /* Insert a cleanup marker for gimplify_cleanup_point_expr.  CLEANUP
is the cleanup action required.  EH_ONLY is true if the cleanup should
-   only be executed if an exception is thrown, not on normal exit.  */
+   only be executed if an exception is thrown, not on normal exit.
+   If FORCE_UNCOND is true perform the cleanup unconditionally;  this is
+   only valid for clobbers.  */

 static void
-gimple_push_cleanup (tree var, tree cleanup, bool eh_only, gimple_seq *pre_p)
+gimple_push_cleanup (tree var, tree cleanup, bool eh_only, gimple_seq *pre_p,
+bool force_uncond = false)
 {
   gimple *wce;
   gimple_seq cleanup_stmts = NULL;
@@ -6301,7 +6304,8 @@ gimple_push_cleanup (tree var, tree clea
   if (seen_error ())
 return;

-  if (gimple_conditional_context ())
+  if (gimple_conditional_context ()
+  && ! force_uncond)
 {
   /* If we're in a conditional context, this is more complex.  We only
 want to run the cleanup if we actually ran the initialization that
@@ -6426,11 +6430,7 @@ gimplify_target_expr (tree *expr_p, gimp
NULL);
  TREE_THIS_VOLATILE (clobber) = true;
  clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
- if (cleanup)
-   cleanup = build2 (COMPOUND_EXPR, void_type_node, cleanup,
- clobber);
- else
-   cleanup = clobber;
+ gimple_push_cleanup (temp, clobber, false, pre_p, true);
}
  if (asan_poisoned_variables && dbg_cnt (asan_use_after_scope))
{

which brings down stack usage to GCC 5 levels (so is even better than the
other patch).

[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |rguenth at gcc dot 
gnu.org

--- Comment #6 from Richard Biener  ---
Created attachment 40991
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40991=edit
revised patch

Fixed patch, previous one caused some miscompile.  Testing in progress (ISTR
seeing some optimization testcases failing as well).

[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[jan.smets at nokia dot com]

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

--- Comment #5 from Jan Smets  ---
I think suggested patch might generate bad code. (but hard to track down on my
embedded target, it's stuck at a point where I don't have exceptions/backtraces
available yet).

[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Priority|P3  |P2
 CC||jakub at gcc dot gnu.org

--- Comment #4 from Richard Biener  ---
The clobbers are on paths founds dead by DCE.  We basically have

   if (cleanup)
 D.2369 ={v} {CLOBBER;}

which, given the clobber isn't really "necessary", gets completely removed.

We can't simply promote the clobber to happen unconditionally.  Simply leaving
the conditional around like with

Index: gcc/tree-ssa-dce.c
===
--- gcc/tree-ssa-dce.c  (revision 246082)
+++ gcc/tree-ssa-dce.c  (working copy)
@@ -182,6 +182,8 @@ mark_operand_necessary (tree op)
   worklist.safe_push (stmt);
 }

+static void
+mark_control_dependent_edges_necessary (basic_block bb, bool ignore_self);

 /* Mark STMT as necessary if it obviously is.  Add it to the worklist if
it can make other statements necessary.
@@ -277,7 +279,11 @@ mark_stmt_if_obviously_necessary (gimple

 case GIMPLE_ASSIGN:
   if (gimple_clobber_p (stmt))
-   return;
+   {
+ if (aggressive)
+   mark_control_dependent_edges_necessary (gimple_bb (stmt), true);
+ return;
+   }
   break;

 default:

results in us eventually doing the clobber unconditionally anyway through
threading and we generate larger code:

> g++-7 -c t.ii -O2
> size t.o
   textdata bss dec hex filename
   1384   0   01384 568 t.o
> g++-7 -c t.ii -O2 -B.
> size t.o
   textdata bss dec hex filename
   1479   0   01479 5c7 t.o

but

> g++-7 -c t.ii -O2 -B. -fstack-usage -Wstack-usage=96
t.ii: In function ‘void test::FN(test::db::ManagedObject&, const
test::BitMask&’:
t.ii:31:6: warning: stack usage is 112 bytes [-Wstack-usage=]
 void FN(ManagedObject , const BitMask &) {
  ^~

which is a bit closer to the 96 bytes needed with gcc5 (which needed a text
size of only 1097 bytes btw, GCC 6 needs 1231 bytes).  With -Os and the patch
trunk needs 80 bytes stack and 1379 bytes text.

I think w/o the threading happening we cannot optimize stack usage later.

Jakub, any opinion on the above patch?

[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski  ---
cddce1 removes them for -fno-exceptions case.

[Bug tree-optimization/80032] [6/7 Regression] C++ excessive stack usage (no stack reuse)

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski  ---
(In reply to Richard Biener from comment #1) 
> For trunk ehcleanup1 removes them.

Though with -fno-exceptions, they are gone before hand.