[committed] amdgcn: Remove omp_gcn pass

Message ID 91e363ad-24cf-e25a-eaae-f75694e80e91@codesourcery.com
State New
Headers show
Series
  • [committed] amdgcn: Remove omp_gcn pass
Related show

Commit Message

Andrew Stubbs Sept. 18, 2020, 10:27 a.m.
This patch removes the amdgcn-specific "omp_gcn" pass that was 
responsible for tweaking the OpenMP middle-end IR for GCN.

In the past there were a few things there to make it work for simple 
cases while real support was built out in the backend and libgomp, but 
those haven't been needed ever since the code was upstreamed.

The only remaining content was replacing two libgomp function calls with 
compiler builtins. Specifically, omp_get_thread_num and omp_get_team_num 
are replaced with the HSA/ROCm grid numbers stored in registers. This is 
an optimization to avoid the function call only, and served no 
correctness purpose because the libgomp functions do work too.

However, I have now learned that OpenMP allows "parallel" regions inside 
other "parallel" regions, and when this happens the team and thread 
numbers no longer match the runtime grid numbers, so the optimization is 
invalid.

Removing the whole pass fixes the correctness issue. It would be nice to 
keep the optimization for the normal case where the regions are not 
nested, but since such calls do not usually occur in the "hot" code, 
it's not clear to me how to detect if nesting is present, and I don't 
have a lot of time to fix this issue, I'm just going to let it go, for now.

Committed to master. I'll backport it to GCC 10 and OG10 shortly.

Andrew

Patch

amdgcn: Remove omp_gcn pass

This pass only had an optimization for obtaining team/thread numbers in it,
and that turns out to be invalid in the presence of nested parallel regions,
so we can simply delete the whole thing.

Of course, it would be nice to apply the optimization where it is valid, but
that will take more effort than I have to spend right now.

gcc/ChangeLog:

	* config/gcn/gcn-tree.c (execute_omp_gcn): Delete.
	(make_pass_omp_gcn): Delete.
	* config/gcn/t-gcn-hsa (PASSES_EXTRA): Delete.
	* config/gcn/gcn-passes.def: Removed.

diff --git a/gcc/config/gcn/gcn-passes.def b/gcc/config/gcn/gcn-passes.def
deleted file mode 100644
index bcf928dd418..00000000000
--- a/gcc/config/gcn/gcn-passes.def
+++ /dev/null
@@ -1,19 +0,0 @@ 
-/* Copyright (C) 2017-2020 Free Software Foundation, Inc.
-
-   This file is part of GCC.
-   
-   GCC is free software; you can redistribute it and/or modify it under
-   the terms of the GNU General Public License as published by the Free
-   Software Foundation; either version 3, or (at your option) any later
-   version.
-   
-   GCC is distributed in the hope that it will be useful, but WITHOUT ANY
-   WARRANTY; without even the implied warranty of MERCHANTABILITY or
-   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
-   for more details.
-   
-   You should have received a copy of the GNU General Public License
-   along with GCC; see the file COPYING3.  If not see
-   <http://www.gnu.org/licenses/>.  */
-
-INSERT_PASS_AFTER (pass_omp_target_link, 1, pass_omp_gcn);
diff --git a/gcc/config/gcn/gcn-tree.c b/gcc/config/gcn/gcn-tree.c
index a681426139b..4304f13160a 100644
--- a/gcc/config/gcn/gcn-tree.c
+++ b/gcc/config/gcn/gcn-tree.c
@@ -45,125 +45,6 @@ 
 #include "targhooks.h"
 #include "langhooks-def.h"
 
-/* }}}  */
-/* {{{ OMP GCN pass.
- 
-   This pass is intended to make any GCN-specfic transformations to OpenMP
-   target regions.
- 
-   At present, its only purpose is to convert some "omp" built-in functions
-   to use closer-to-the-metal "gcn" built-in functions.  */
-
-unsigned int
-execute_omp_gcn (void)
-{
-  tree thr_num_tree = builtin_decl_explicit (BUILT_IN_OMP_GET_THREAD_NUM);
-  tree thr_num_id = DECL_NAME (thr_num_tree);
-  tree team_num_tree = builtin_decl_explicit (BUILT_IN_OMP_GET_TEAM_NUM);
-  tree team_num_id = DECL_NAME (team_num_tree);
-  basic_block bb;
-  gimple_stmt_iterator gsi;
-  unsigned int todo = 0;
-
-  FOR_EACH_BB_FN (bb, cfun)
-    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-    {
-      gimple *call = gsi_stmt (gsi);
-      tree decl;
-
-      if (is_gimple_call (call) && (decl = gimple_call_fndecl (call)))
-	{
-	  tree decl_id = DECL_NAME (decl);
-	  tree lhs = gimple_get_lhs (call);
-
-	  if (decl_id == thr_num_id)
-	    {
-	      if (dump_file && (dump_flags & TDF_DETAILS))
-		fprintf (dump_file,
-			 "Replace '%s' with __builtin_gcn_dim_pos.\n",
-			 IDENTIFIER_POINTER (decl_id));
-
-	      /* Transform this:
-	         lhs = __builtin_omp_get_thread_num ()
-	         to this:
-	         lhs = __builtin_gcn_dim_pos (1)  */
-	      tree fn = targetm.builtin_decl (GCN_BUILTIN_OMP_DIM_POS, 0);
-	      tree fnarg = build_int_cst (unsigned_type_node, 1);
-	      gimple *stmt = gimple_build_call (fn, 1, fnarg);
-	      gimple_call_set_lhs (stmt, lhs);
-	      gsi_replace (&gsi, stmt, true);
-
-	      todo |= TODO_update_ssa;
-	    }
-	  else if (decl_id == team_num_id)
-	    {
-	      if (dump_file && (dump_flags & TDF_DETAILS))
-		fprintf (dump_file,
-			 "Replace '%s' with __builtin_gcn_dim_pos.\n",
-			 IDENTIFIER_POINTER (decl_id));
-
-	      /* Transform this:
-	         lhs = __builtin_omp_get_team_num ()
-	         to this:
-	         lhs = __builtin_gcn_dim_pos (0)  */
-	      tree fn = targetm.builtin_decl (GCN_BUILTIN_OMP_DIM_POS, 0);
-	      tree fnarg = build_zero_cst (unsigned_type_node);
-	      gimple *stmt = gimple_build_call (fn, 1, fnarg);
-	      gimple_call_set_lhs (stmt, lhs);
-	      gsi_replace (&gsi, stmt, true);
-
-	      todo |= TODO_update_ssa;
-	    }
-	}
-    }
-
-  return todo;
-}
-
-namespace
-{
-
-  const pass_data pass_data_omp_gcn = {
-    GIMPLE_PASS,
-    "omp_gcn",			/* name */
-    OPTGROUP_NONE,		/* optinfo_flags */
-    TV_NONE,			/* tv_id */
-    0,				/* properties_required */
-    0,				/* properties_provided */
-    0,				/* properties_destroyed */
-    0,				/* todo_flags_start */
-    TODO_df_finish,		/* todo_flags_finish */
-  };
-
-  class pass_omp_gcn : public gimple_opt_pass
-  {
-  public:
-    pass_omp_gcn (gcc::context *ctxt)
-      : gimple_opt_pass (pass_data_omp_gcn, ctxt)
-    {
-    }
-
-    /* opt_pass methods: */
-    virtual bool gate (function *)
-    {
-      return flag_openmp;
-    }
-
-    virtual unsigned int execute (function *)
-    {
-      return execute_omp_gcn ();
-    }
-
-  }; /* class pass_omp_gcn.  */
-
-} /* anon namespace.  */
-
-gimple_opt_pass *
-make_pass_omp_gcn (gcc::context *ctxt)
-{
-  return new pass_omp_gcn (ctxt);
-}
-
 /* }}}  */
 /* {{{ OpenACC reductions.  */
 
diff --git a/gcc/config/gcn/t-gcn-hsa b/gcc/config/gcn/t-gcn-hsa
index af203c5cf05..16d243c3f2b 100644
--- a/gcc/config/gcn/t-gcn-hsa
+++ b/gcc/config/gcn/t-gcn-hsa
@@ -45,7 +45,6 @@  gcn-run$(exeext): gcn-run.o
 MULTILIB_OPTIONS = march=gfx900/march=gfx906
 MULTILIB_DIRNAMES = gfx900 gfx906
 
-PASSES_EXTRA += $(srcdir)/config/gcn/gcn-passes.def
 gcn-tree.o: $(srcdir)/config/gcn/gcn-tree.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)