[04/14] Add {symbol,call}_summary::get method and use it in HSA.

Message ID 7c009cef6fc77f30fc0d65c3e45f47bbdbbfefd3.1526551813.git.mliska@suse.cz
State New
Headers show
Series
  • Finish transition of {symbol,call}_summary.
Related show

Commit Message

Martin Liška April 20, 2018, 8:25 a.m.
gcc/ChangeLog:

2018-04-24  Martin Liska  <mliska@suse.cz>

	* hsa-common.h (enum hsa_function_kind): Rename HSA_NONE to
	HSA_INVALID.
	(hsa_function_summary::hsa_function_summary): Use the new enum
	value.
	(hsa_gpu_implementation_p): Use hsa_summaries::get.
	* hsa-gen.c (hsa_get_host_function): Likewise.
	(get_brig_function_name): Likewise.
	* ipa-hsa.c (process_hsa_functions): Likewise.
	(ipa_hsa_write_summary): Likewise.
	* symbol-summary.h (symtab_duplication): Use ::get function/
	(get): New function.
---
 gcc/hsa-common.h     | 15 ++++++++-------
 gcc/hsa-gen.c        |  9 +++------
 gcc/ipa-hsa.c        | 22 +++++++++++-----------
 gcc/symbol-summary.h | 40 ++++++++++++++++++++++++++--------------
 4 files changed, 48 insertions(+), 38 deletions(-)

Comments

Jan Hubicka June 7, 2018, 12:06 p.m. | #1
> 

> gcc/ChangeLog:

> 

> 2018-04-24  Martin Liska  <mliska@suse.cz>

> 

> 	* hsa-common.h (enum hsa_function_kind): Rename HSA_NONE to

> 	HSA_INVALID.

> 	(hsa_function_summary::hsa_function_summary): Use the new enum

> 	value.

> 	(hsa_gpu_implementation_p): Use hsa_summaries::get.

> 	* hsa-gen.c (hsa_get_host_function): Likewise.

> 	(get_brig_function_name): Likewise.

> 	* ipa-hsa.c (process_hsa_functions): Likewise.

> 	(ipa_hsa_write_summary): Likewise.

> 	* symbol-summary.h (symtab_duplication): Use ::get function/

> 	(get): New function.


OK if Martin (Jambor) is happy with HSA changes :)

Honza
Martin Jambor June 7, 2018, 12:37 p.m. | #2
On Thu, Jun 07 2018, Jan Hubicka wrote:
>> 

>> gcc/ChangeLog:

>> 

>> 2018-04-24  Martin Liska  <mliska@suse.cz>

>> 

>> 	* hsa-common.h (enum hsa_function_kind): Rename HSA_NONE to

>> 	HSA_INVALID.

>> 	(hsa_function_summary::hsa_function_summary): Use the new enum

>> 	value.

>> 	(hsa_gpu_implementation_p): Use hsa_summaries::get.

>> 	* hsa-gen.c (hsa_get_host_function): Likewise.

>> 	(get_brig_function_name): Likewise.

>> 	* ipa-hsa.c (process_hsa_functions): Likewise.

>> 	(ipa_hsa_write_summary): Likewise.

>> 	* symbol-summary.h (symtab_duplication): Use ::get function/

>> 	(get): New function.

>

> OK if Martin (Jambor) is happy with HSA changes :)

>


I assume this is the version which I have tested for Martin (and which
passed all tests).  So OK from me too.

Thanks,

Martin

Patch

diff --git a/gcc/hsa-common.h b/gcc/hsa-common.h
index 849363c7b49..c72343fbdab 100644
--- a/gcc/hsa-common.h
+++ b/gcc/hsa-common.h
@@ -1208,7 +1208,7 @@  public:
 
 enum hsa_function_kind
 {
-  HSA_NONE,
+  HSA_INVALID,
   HSA_KERNEL,
   HSA_FUNCTION
 };
@@ -1234,7 +1234,7 @@  struct hsa_function_summary
 };
 
 inline
-hsa_function_summary::hsa_function_summary (): m_kind (HSA_NONE),
+hsa_function_summary::hsa_function_summary (): m_kind (HSA_INVALID),
   m_bound_function (NULL), m_gpu_implementation_p (false)
 {
 }
@@ -1244,7 +1244,10 @@  class hsa_summary_t: public function_summary <hsa_function_summary *>
 {
 public:
   hsa_summary_t (symbol_table *table):
-    function_summary<hsa_function_summary *> (table) { }
+    function_summary<hsa_function_summary *> (table)
+  {
+    disable_insertion_hook ();
+  }
 
   /* Couple GPU and HOST as gpu-specific and host-specific implementation of
      the same function.  KIND determines whether GPU is a host-invokable kernel
@@ -1407,10 +1410,8 @@  hsa_gpu_implementation_p (tree decl)
   if (hsa_summaries == NULL)
     return false;
 
-  hsa_function_summary *s
-    = hsa_summaries->get_create (cgraph_node::get_create (decl));
-
-  return s->m_gpu_implementation_p;
+  hsa_function_summary *s = hsa_summaries->get (cgraph_node::get_create (decl));
+  return s != NULL && s->m_gpu_implementation_p;
 }
 
 #endif /* HSA_H */
diff --git a/gcc/hsa-gen.c b/gcc/hsa-gen.c
index b573bd57d20..5f9feb31067 100644
--- a/gcc/hsa-gen.c
+++ b/gcc/hsa-gen.c
@@ -961,9 +961,7 @@  get_symbol_for_decl (tree decl)
 tree
 hsa_get_host_function (tree decl)
 {
-  hsa_function_summary *s
-    = hsa_summaries->get_create (cgraph_node::get_create (decl));
-  gcc_assert (s->m_kind != HSA_NONE);
+  hsa_function_summary *s = hsa_summaries->get (cgraph_node::get_create (decl));
   gcc_assert (s->m_gpu_implementation_p);
 
   return s->m_bound_function ? s->m_bound_function->decl : NULL;
@@ -976,9 +974,8 @@  get_brig_function_name (tree decl)
 {
   tree d = decl;
 
-  hsa_function_summary *s
-    = hsa_summaries->get_create (cgraph_node::get_create (d));
-  if (s->m_kind != HSA_NONE
+  hsa_function_summary *s = hsa_summaries->get (cgraph_node::get_create (d));
+  if (s != NULL
       && s->m_gpu_implementation_p
       && s->m_bound_function)
     d = s->m_bound_function->decl;
diff --git a/gcc/ipa-hsa.c b/gcc/ipa-hsa.c
index 90d193fe517..7cfeba08e58 100644
--- a/gcc/ipa-hsa.c
+++ b/gcc/ipa-hsa.c
@@ -76,13 +76,13 @@  process_hsa_functions (void)
 
   FOR_EACH_DEFINED_FUNCTION (node)
     {
-      hsa_function_summary *s = hsa_summaries->get_create (node);
+      hsa_function_summary *s = hsa_summaries->get (node);
 
       /* A linked function is skipped.  */
-      if (s->m_bound_function != NULL)
+      if (s != NULL && s->m_bound_function != NULL)
 	continue;
 
-      if (s->m_kind != HSA_NONE)
+      if (s != NULL)
 	{
 	  if (!check_warn_node_versionable (node))
 	    continue;
@@ -130,11 +130,11 @@  process_hsa_functions (void)
 
       while (e)
 	{
-	  hsa_function_summary *src = hsa_summaries->get_create (node);
-	  if (src->m_kind != HSA_NONE && src->m_gpu_implementation_p)
+	  hsa_function_summary *src = hsa_summaries->get (node);
+	  if (src != NULL && src->m_gpu_implementation_p)
 	    {
-	      hsa_function_summary *dst = hsa_summaries->get_create (e->callee);
-	      if (dst->m_kind != HSA_NONE && !dst->m_gpu_implementation_p)
+	      hsa_function_summary *dst = hsa_summaries->get (e->callee);
+	      if (dst != NULL && !dst->m_gpu_implementation_p)
 		{
 		  e->redirect_callee (dst->m_bound_function);
 		  if (dump_file)
@@ -174,9 +174,9 @@  ipa_hsa_write_summary (void)
        lsei_next_function_in_partition (&lsei))
     {
       node = lsei_cgraph_node (lsei);
-      hsa_function_summary *s = hsa_summaries->get_create (node);
+      hsa_function_summary *s = hsa_summaries->get (node);
 
-      if (s->m_kind != HSA_NONE)
+      if (s != NULL)
 	count++;
     }
 
@@ -187,9 +187,9 @@  ipa_hsa_write_summary (void)
        lsei_next_function_in_partition (&lsei))
     {
       node = lsei_cgraph_node (lsei);
-      hsa_function_summary *s = hsa_summaries->get_create (node);
+      hsa_function_summary *s = hsa_summaries->get (node);
 
-      if (s->m_kind != HSA_NONE)
+      if (s != NULL)
 	{
 	  encoder = ob->decl_state->symtab_node_encoder;
 	  int node_ref = lto_symtab_encoder_encode (encoder, node);
diff --git a/gcc/symbol-summary.h b/gcc/symbol-summary.h
index 7ba769739ee..8227abbf210 100644
--- a/gcc/symbol-summary.h
+++ b/gcc/symbol-summary.h
@@ -90,8 +90,13 @@  public:
      does not exist it will be created.  */
   T* get_create (cgraph_node *node)
   {
-    gcc_checking_assert (node->summary_uid);
-    return get_create (node->summary_uid);
+    return get (node->summary_uid, true);
+  }
+
+  /* Getter for summary callgraph node pointer.  */
+  T* get (cgraph_node *node)
+  {
+    return get (node->summary_uid, false);
   }
 
   /* Return number of elements handled by data structure.  */
@@ -136,7 +141,7 @@  private:
   typedef int_hash <int, 0, -1> map_hash;
 
   /* Getter for summary callgraph ID.  */
-  T* get_create (int uid);
+  T *get (int uid, bool lazy_insert);
 
   /* Indicates if insertion hook is enabled.  */
   bool m_insertion_enabled;
@@ -245,30 +250,37 @@  function_summary<T *>::symtab_duplication (cgraph_node *node,
 					   cgraph_node *node2, void *data)
 {
   function_summary *summary = (function_summary <T *> *) (data);
-  T **v = summary->m_map.get (node->summary_uid);
-
-  gcc_checking_assert (node2->summary_uid > 0);
+  T *v = summary->get (node);
 
   if (v)
     {
       /* This load is necessary, because we insert a new value!  */
-      T *data = *v;
       T *duplicate = summary->allocate_new ();
       summary->m_map.put (node2->summary_uid, duplicate);
-      summary->duplicate (node, node2, data, duplicate);
+      summary->duplicate (node, node2, v, duplicate);
     }
 }
 
 template <typename T>
 T*
-function_summary<T *>::get_create (int uid)
+function_summary<T *>::get (int uid, bool lazy_insert)
 {
-  bool existed;
-  T **v = &m_map.get_or_insert (uid, &existed);
-  if (!existed)
-    *v = allocate_new ();
+  gcc_checking_assert (uid > 0);
 
-  return *v;
+  if (lazy_insert)
+    {
+      bool existed;
+      T **v = &m_map.get_or_insert (uid, &existed);
+      if (!existed)
+	*v = allocate_new ();
+
+      return *v;
+    }
+  else
+    {
+      T **v = m_map.get (uid);
+      return v == NULL ? NULL : *v;
+    }
 }
 
 template <typename T>