Hi Tom!

On Thu, 24 Sep 2015 08:36:27 +0200, Tom de Vries <tom_devr...@mentor.com> wrote:
> On 24/09/15 08:23, Thomas Schwinge wrote:
> > On Tue, 11 Aug 2015 20:53:39 +0200, Tom de Vries <tom_devr...@mentor.com> 
> > wrote:
> >> Don't create superfluous parm in expand_omp_taskreg
> >>
> >> 2015-08-11  Tom de Vries  <t...@codesourcery.com>
> >>
> >>    * omp-low.c (expand_omp_taskreg): If in ssa, set rhs of parcopy stmt to
> >>    parm_decl, rather than generating a dummy default def in cfun.
> >>    * tree-cfg.c (replace_ssa_name): Assume no default defs.  Make sure
> >>    ssa_name from cfun and child_fn do not share a stmt as def stmt.
> >>    (move_stmt_op): Handle PARM_DECl.
> >>    (gather_ssa_name_hash_map_from): New function.
> >>    (move_sese_region_to_fn): Add default defs for function params, and add
> >>    them to vars_map.  Release copied ssa names.
> >>    * tree-cfg.h (gather_ssa_name_hash_map_from): Declare.
> >
> > Do I understand correct that with this change present on trunk (which I'm
> > currently merging into gomp-4_0-branch), the changes you've earlier done
> > on gomp-4_0-branch to gcc/omp-low.c:release_dangling_ssa_names,
> > gcc/tree-cfg.c:replace_ssa_name, should now be reverted?  That is, how
> > much of the following patches can be reverted now (listed backwards in
> > time)?
> 
> indeed, in the above commit we release the dangling ssa names in 
> move_sese_region_to_fn. So after committing this patch to the 
> gomp-4_0-branch, the call to release_dangling_ssa_names is no longer 
> necessary, and the function release_dangling_ssa_names can be removed.

From IRC:

    <tschwinge> vries: Are you totally busy right now, or could you spend
      an hour on backporting to gomp-4_0-branch your trunk commit that I
      mentorioned earlier today?
    <vries> tschwinge: shouldn't be a problem
    <tschwinge> veWell, I'm asking because in my merge tree, I'm running
      into an assertion that you added there -- not sure yet whether I've
      done something wrong, though.
    <tschwinge> vries: ^
    <vries> tschwinge: it would be useful for me to know which assertion

So far, I only looked at libgomp test results, and there, none of the
OpenMP but quite a number of the OpenACC tests fail as follows, for
example libgomp.oacc-c-c++-common/kernels-1.c:

    [...]/build-gcc/gcc/xgcc -B[...]/build-gcc/gcc/ 
[...]/source-gcc/libgomp/testsuite/libgomp.oacc-c/../libgomp.oacc-c-c++-common/kernels-1.c
 -B[...]/build-gcc/x86_64-pc-linux-gnu/./libgomp/ 
-B[...]/build-gcc/x86_64-pc-linux-gnu/./libgomp/.libs 
-I[...]/build-gcc/x86_64-pc-linux-gnu/./libgomp 
-I[...]/source-gcc/libgomp/testsuite/../../include 
-I[...]/source-gcc/libgomp/testsuite/.. 
-I/usr/local/cuda-5.5/targets/x86_64-linux/include -fmessage-length=0 
-fno-diagnostics-show-caret -fdiagnostics-color=never 
-B[...]/install/offload-nvptx-none/libexec/gcc/x86_64-pc-linux-gnu/6.0.0 
-B[...]/install/offload-nvptx-none/bin 
-B[...]/install/offload-x86_64-intelmicemul-linux-gnu/libexec/gcc/x86_64-pc-linux-gnu/6.0.0
 -B[...]/install/offload-x86_64-intelmicemul-linux-gnu/bin -fopenacc 
-I[...]/source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common 
-DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 -foffload=nvptx-none -O2 
-L[...]/build-gcc/x86_64-pc-linux-gnu/./libgomp/.libs -lm -o ./kernels-1.exe
    In file included from 
[...]/source-gcc/libgomp/testsuite/libgomp.oacc-c/../libgomp.oacc-c-c++-common/kernels-1.c:6:0:
    
[...]/source-gcc/libgomp/testsuite/libgomp.oacc-c/../libgomp.oacc-c-c++-common/data-clauses.h:
 In function 'main':
    
[...]/source-gcc/libgomp/testsuite/libgomp.oacc-c/../libgomp.oacc-c-c++-common/data-clauses.h:182:9:
 internal compiler error: in replace_ssa_name, at tree-cfg.c:6423
    0xb0518a replace_ssa_name
            [...]/source-gcc/gcc/tree-cfg.c:6423
    0xb05407 move_stmt_op
            [...]/source-gcc/gcc/tree-cfg.c:6501
    0xd75e43 walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*, void*), 
void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*, tree_node* 
(*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*), void*, 
hash_set<tree_node*, default_hash_traits<tree_node*> >*))
            [...]/source-gcc/gcc/tree.c:11341
    0x89832c walk_gimple_op(gimple*, tree_node* (*)(tree_node**, int*, void*), 
walk_stmt_info*)
            [...]/source-gcc/gcc/gimple-walk.c:204
    0x898814 walk_gimple_stmt(gimple_stmt_iterator*, tree_node* 
(*)(gimple_stmt_iterator*, bool*, walk_stmt_info*), tree_node* (*)(tree_node**, 
int*, void*), walk_stmt_info*)
            [...]/source-gcc/gcc/gimple-walk.c:562
    0xb088e3 move_block_to_fn
            [...]/source-gcc/gcc/tree-cfg.c:6774
    0xb088e3 move_sese_region_to_fn(function*, basic_block_def*, 
basic_block_def*, tree_node*)
            [...]/source-gcc/gcc/tree-cfg.c:7238
    0x9c1133 expand_omp_target
            [...]/source-gcc/gcc/omp-low.c:9802
    0x9c3a1c expand_omp
            [...]/source-gcc/gcc/omp-low.c:10240
    0x9cc3fe execute_expand_omp
            [...]/source-gcc/gcc/omp-low.c:10486
    0x9cc568 execute
            [...]/source-gcc/gcc/omp-low.c:10609

source-gcc/gcc/tree-cfg.c:

    [...]
      6405  /* Creates an ssa name in TO_CONTEXT equivalent to NAME.
      6406     VARS_MAP maps old ssa names and var_decls to the new ones.  */
      6407  
      6408  static tree
      6409  replace_ssa_name (tree name, hash_map<tree, tree> *vars_map,
      6410                    tree to_context)
      6411  {
      6412    tree new_name;
      6413  
      6414    gcc_assert (!virtual_operand_p (name));
      6415  
      6416    tree *loc = vars_map->get (name);
      6417  
      6418    if (!loc)
      6419      {
      6420        tree decl = SSA_NAME_VAR (name);
      6421        if (decl)
      6422          {
      6423            gcc_assert (!SSA_NAME_IS_DEFAULT_DEF (name));
      6424            replace_by_duplicate_decl (&decl, vars_map, to_context);
      6425            /* If name is a default def, then we don't move the 
defining stmt
      6426               (which is a nop).  Because (1) the nop doesn't belong 
to the sese
      6427               region, and (2) while setting the def stmt of name to 
NULL would
      6428               trigger release_ssa_name in 
release_dangling_ssa_names, it wouldn't
      6429               be released since it's a default def, and subsequently 
cause an
      6430               ssa verification failure.  */
      6431            new_name = make_ssa_name_fn (DECL_STRUCT_FUNCTION 
(to_context),
      6432                                         decl, SSA_NAME_DEF_STMT 
(name));
      6433            if (SSA_NAME_IS_DEFAULT_DEF (name))
      6434              set_ssa_default_def (DECL_STRUCT_FUNCTION (to_context),
      6435                                   decl, new_name);
      6436          }
      6437        else
      6438          new_name = copy_ssa_name_fn (DECL_STRUCT_FUNCTION 
(to_context),
      6439                                       name, SSA_NAME_DEF_STMT 
(name));
      6440  
      6441        /* Now that we've used the def stmt to define new_name, make 
sure it
      6442           doesn't define name anymore.  */
      6443        SSA_NAME_DEF_STMT (name) = NULL;
      6444  
      6445        vars_map->put (name, new_name);
      6446  
      6447        if (!SSA_NAME_IS_DEFAULT_DEF (name))
      6448          /* The statement has been moved to the child function.  It 
no longer
      6449             defines name in the original function.  Mark the def 
stmt NULL, and
      6450             let release_dangling_ssa_names deal with it.  */
      6451          SSA_NAME_DEF_STMT (name) = NULL;
      6452      }
      6453    else
      6454      new_name = *loc;
      6455  
      6456    return new_name;
      6457  }
    [...]

As I said, this is in my WIP tree to merge from trunk into
gomp-4_0-branch.  To rule out any obvious things, would you please
backport to gomp-4_0-branch your trunk commit r227103
(883f001d2c3672e0674bec71f36a2052734a72cf, "Don't create superfluous parm
in expand_omp_taskreg"), and revert the following changes as appropriate:

> > commit 6befb84f4c0157a4cdf66cfaf64e457180f9a7fa
> > Author: vries <vries@138bc75d-0d04-0410-961f-82ee72b054a4>
> > Date:   Wed Aug 5 06:01:08 2015 +0000
> >
> >      Fix release_dangling_ssa_names
> >
> >      2015-08-05  Tom de Vries  <t...@codesourcery.com>
> >
> >          * omp-low.c (release_dangling_ssa_names): Release SSA_NAMEs with 
> > NULL
> >          def stmt.
> >          * tree-cfg.c (replace_ssa_name): Don't move default def nops.  Set 
> > def
> >          stmt of unused SSA_NAME to NULL.
> >
> >      git-svn-id: 
> > svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@226608 
> > 138bc75d-0d04-0410-961f-82ee72b054a4
> >
> > commit 0cf67438bd87e5a6ec063e90da0ea20801bda54c
> > Author: vries <vries@138bc75d-0d04-0410-961f-82ee72b054a4>
> > Date:   Thu Jun 4 15:47:09 2015 +0000
> >
> >      Add release_dangling_ssa_names
> >
> >      2015-06-04  Tom de Vries  <t...@codesourcery.com>
> >
> >          * omp-low.c (release_dangling_ssa_names): Factor out of ...
> >          (pass_expand_omp_ssa::execute): ... here.
> >
> >      git-svn-id: 
> > svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@224130 
> > 138bc75d-0d04-0410-961f-82ee72b054a4
> >
> > commit 93557ac5e30c26ee1a3d1255e31265b287171a0d
> > Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
> > Date:   Tue Apr 21 19:37:19 2015 +0000
> >
> >      Expand oacc kernels after pass_fre
> >
> >          gcc/
> >          * omp-low.c: Include gimple-pretty-print.h.
> >          (release_first_vuse_in_edge_dest): New function.
> >          (expand_omp_target): When not in ssa, don't split off oacc kernels
> >          region, clear PROP_gimple_eomp in cfun->curr_properties to force 
> > later
> >          expanssion, and add GOACC_kernels_internal call.
> >          When in ssa, split off oacc kernels and convert 
> > GOACC_kernels_internal
> >          into GOACC_kernels call.  Handle ssa-code.
> >          (pass_data_expand_omp): Don't set PROP_gimple_eomp unconditionally 
> > in
> >          properties_provided field.
> >          (pass_expand_omp::execute): Set PROP_gimple_eomp in
> >          cfun->curr_properties tentatively.
> >          (pass_data_expand_omp_ssa): Add TODO_remove_unused_locals to
> >          todo_flags_finish field.
> >          (pass_expand_omp_ssa::execute): Release dangling SSA_NAMEs after 
> > calling
> >          execute_expand_omp.
> >          (gimple_stmt_ssa_operand_references_var_p)
> >          (gimple_stmt_omp_data_i_init_p): New function.
> >          * omp-low.h (gimple_stmt_omp_data_i_init_p): Declare.
> >          * passes.def: Add pass_expand_omp_ssa after pass_fre.  Add
> >          pass_expand_omp_ssa after pass_all_early_optimizations.
> >          * tree-ssa-ccp.c: Include omp-low.h.
> >          (surely_varying_stmt_p, ccp_visit_stmt): Handle .omp_data_i init
> >          conservatively.
> >          * tree-ssa-forwprop.c: Include omp-low.h.
> >          (pass_forwprop::execute): Handle .omp_data_i init conservatively.
> >          * tree-ssa-sccvn.c: Include omp-low.h.
> >          (visit_use): Handle .omp_data_i init conservatively.
> >          * cgraph.c (cgraph_node::release_body): Don't release offloadable
> >          functions.
> >
> >      git-svn-id: 
> > svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@222279 
> > 138bc75d-0d04-0410-961f-82ee72b054a4


Grüße,
 Thomas

Attachment: signature.asc
Description: PGP signature

Reply via email to