[Bug c++/34610] New: [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[Bug c++/34610] New: [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[reichelt at gcc dot gnu dot org]

The following valid code snippet triggers an ICE on mainline when compiled
with "-fprofile-arcs -fopenmp":

=======================================================
void foo()
{
#pragma omp for
  for (int i=0; i<10; ++i)
    ;
}
=======================================================

bug.cc: In function 'void foo()':
bug.cc:3: internal compiler error: in expand_omp_for_static_nochunk, at
omp-low.c:2916
Please submit a full bug report, [etc.]

If I add "schedule(dynamic)", I get a slightly different error:

=======================================================
void foo()
{
#pragma omp for schedule(dynamic)
  for (int i=0; i<10; ++i)
    ;
}
=======================================================

bug.cc: In function 'void foo()':
bug.cc:3: internal compiler error: in expand_omp_for_generic, at omp-low.c:2714
Please submit a full bug report, [etc.]


-- 
           Summary: [4.3 regression] ICE with "-fprofile-arcs -fopenmp"
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code, monitored, openmp
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: reichelt at gcc dot gnu dot org


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

[Bug c++/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[reichelt at gcc dot gnu dot org]

-- 

reichelt at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.3.0


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2007-12-28 20:16 -------
It should be noted that -fprofile-arcs is not thread safe yet anyways so at
runtime, you will get weird answers.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu dot
                   |                            |org
          Component|c++                         |gcov-profile


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[jakub at gcc dot gnu dot org]

------- Comment #3 from jakub at gcc dot gnu dot org  2008-01-03 15:01 -------
Additionally, pass_tree_profile is executed multiple times with -fopenmp, e.g.
on
void foo (int i)
{
  #pragma omp parallel
  if (i > 2)
    bar (i + 1);
  else if (i < -2)
    bar (i / 2);
  else
    bar (i * 2);
}
(which compiles) profile counters are added before omp expansion and once again
for the child function.  Is there a reason why pass_tree_profile can't run
after
pass_cleanup_cfg/pass_init_datastructures/pass_expand_omp?
If it can't be moved, could it at least skip all edges in omp parallel regions?
Or if it is not thread safe, just disable pass_tree_profile if flag_openmp...


-- 


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[hubicka at ucw dot cz]

------- Comment #4 from hubicka at ucw dot cz  2008-01-03 21:05 -------
Subject: Re:  [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

> (which compiles) profile counters are added before omp expansion and once again
> for the child function.  Is there a reason why pass_tree_profile can't run
> after
> pass_cleanup_cfg/pass_init_datastructures/pass_expand_omp?

The reason is that pass_cleanup_cfg might remove some explicit goto
statements and gcov output would miss them then.
> If it can't be moved, could it at least skip all edges in omp parallel regions?
> Or if it is not thread safe, just disable pass_tree_profile if flag_openmp...

Well, there is thread safe profiling.  In general tree_profile should
not see anything it profiled already.  I guess all we need to is to mark
clones constructed by OMP and ignore those functions in tree_profile?

Honza


-- 


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[jakub at gcc dot gnu dot org]

------- Comment #5 from jakub at gcc dot gnu dot org  2008-01-04 17:09 -------
That's only part of the problem.  The other is that ompexpand is very upset if
new basic blocks are inserted in unexpected places, e.g. on the BRANCH_EDGE
from OMP_FOR bb or on the FALLTHRU_EDGE from OMP_CONTINUE.  expand_omp_for_*
changes the CFG a lot and so adding profile insns on the edges it will revamp
is premature.
The following patch lets me make -C gcc -k check
RUNTESTFLAGS='--target_board=unix/-fprofile-arcs gomp.exp'
and
make -C */libgomp -k check RUNTESTFLAGS='--target_board=unix/-fprofile-arcs'
(without it there are many ICEs):
--- tree-profile.c.jj16 2007-12-07 12:21:06.000000000 +0100
+++ tree-profile.c      2008-01-04 17:57:28.000000000 +0100
@@ -171,6 +171,19 @@ tree_gen_edge_profiler (int edgeno, edge
 {
   tree ref, one, stmt1, stmt2, stmt3;

+  if (flag_openmp)
+    {
+      tree stmt = last_stmt (e->src);
+      if (stmt)
+       {
+         /* Avoid inserting profiling instructions on edges
+            which OpenMP expand will replace.  */
+         if (TREE_CODE (stmt) == OMP_CONTINUE
+             || TREE_CODE (stmt) == OMP_FOR)
+           return;
+       }
+    }
+
   /* We share one temporary variable declaration per function.  This
      gets re-set in tree_profiling.  */
   if (gcov_type_tmp_var == NULL_TREE)

In addition to this some bit in cfun? to avoid pass_tree_profile would be
needed
and expand_omp_parallel should set that for the child fns it creates.

Is this ok for you?


-- 


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[hubicka at ucw dot cz]

------- Comment #6 from hubicka at ucw dot cz  2008-01-04 17:28 -------
Subject: Re:  [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

> +  if (flag_openmp)
> +    {
> +      tree stmt = last_stmt (e->src);
> +      if (stmt)
> +       {
> +         /* Avoid inserting profiling instructions on edges
> +            which OpenMP expand will replace.  */
> +         if (TREE_CODE (stmt) == OMP_CONTINUE
> +             || TREE_CODE (stmt) == OMP_FOR)
> +           return;

Just not inserting the instrumentation would lead to profile mismatch
and strange errors with -fprofile-use or gcov.  Why those edges are not
marked as ABNORMAL when they can not be split? That would drive
profile.c to not instrument them or if we don't want to ABNORMALize them
for some reason (perhaps we want cleanp_cfg to thread them), then checks
for them in profile.c at same time ABRNORMAL edges are handles is right
thing to do.
> +       }
> +    }
> +
>    /* We share one temporary variable declaration per function.  This
>       gets re-set in tree_profiling.  */
>    if (gcov_type_tmp_var == NULL_TREE)
> 
> In addition to this some bit in cfun? to avoid pass_tree_profile would be
> needed
> and expand_omp_parallel should set that for the child fns it creates.

There is already cfun->profile.

Honza


-- 


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[steven at gcc dot gnu dot org]

------- Comment #7 from steven at gcc dot gnu dot org  2008-01-08 16:26 -------
This is marked as a regression -- but from what? Did this work in GCC 4.2?


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
      Known to fail|                            |4.3.0
   Last reconfirmed|0000-00-00 00:00:00         |2008-01-08 16:26:14
               date|                            |


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2008-01-08 16:45 -------
Yes, this doesn't ICE for me for 4.2.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.2.2


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[jakub at gcc dot gnu dot org]

------- Comment #9 from jakub at gcc dot gnu dot org  2008-01-09 15:05 -------
I've tried to change expand_omp_for_{generic,static_nochunk,static_chunk}
to grok the possibly split edges if profile_arc_flag, but it pretty quickly
gets horribly ugly.  Several edges are removed, split or tweaked by this
expander and if in each case of the 4 problematic critical edges we need to
check
if profiler split that edge or not and deal with it, the code wouldn't be
maintainable.
Can you expand on what kind of issues would occur by not instrumenting them?
That means just the counters will be 0, as nothing will ever increase them,
right?
At least the BRANCH_EDGE of OMP_CONTINUE ending bb will often form a cycle
(whenever the body of the loop contains just single basic block), so treating
them as EDGE_ABNORMAL would just mean adding all those fake edges etc.


-- 


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[jakub at gcc dot gnu dot org]

------- Comment #10 from jakub at gcc dot gnu dot org  2008-01-09 15:55 -------
Actually, marking the edges as EDGE_ABNORMAL in make_edges and clearing that
flag during expand_omp_for time seems to work, so perhaps we can do it that
way.  expand_omp happens before going into SSA, so this won't cause many
SSA_NAMEs are marked as SSA_NAME_OCCURS_IN_ABNORMAL_PHI because of this.

Back to the cfun flag, from what I see cfun->profile isn't useful here,
clearing it means the child function won't be profiled at all.
We want a flag which tells whether tree_profile pass already happened on a
function or not.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |jakub at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-01-08 16:26:14         |2008-01-09 15:55:15
               date|                            |


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[jakub at gcc dot gnu dot org]

------- Comment #11 from jakub at gcc dot gnu dot org  2008-01-19 17:50 -------
Subject: Bug 34610

Author: jakub
Date: Sat Jan 19 17:49:46 2008
New Revision: 131653

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131653
Log:
        PR gcov-profile/34610
        * tree-cfg.c (make_edges): Mark both outgoing edges from
        OMP_CONTINUE and from OMP_FOR as EDGE_ABNORMAL.
        * omp-low.c (expand_omp_for): Clear EDGE_ABNORMAL bits
        from OMP_FOR and OMP_CONTINUE outgoing edges.

        * tree-profile.c (tree_profiling): Return early if
        cfun->after_tree_profile != 0.  Set cfun->after_tree_profile
        at the end.
        * omp-low.c (expand_omp_parallel): Copy after_tree_profile
        from cfun to child_cfun.
        * function.h (struct function): Add after_tree_profile bit.

        * gcc.dg/gomp/pr34610.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/gomp/pr34610.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/function.h
    trunk/gcc/omp-low.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-cfg.c
    trunk/gcc/tree-profile.c


-- 


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

[Bug gcov-profile/34610] [4.3 regression] ICE with "-fprofile-arcs -fopenmp"

[jakub at gcc dot gnu dot org]

------- Comment #12 from jakub at gcc dot gnu dot org  2008-01-19 17:56 -------
Fixed.


-- 

jakub at gcc dot gnu dot org changed:

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


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