diff mbox

[001/236] Convert lab_rtx_for_bb from pointer_map_t to pointer_map<rtx>

Message ID 1407345815-14551-2-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Aug. 6, 2014, 5:19 p.m. UTC
This gives a slight improvement in typesafety in cfgexpand.c

gcc/
	* cfgexpand.c (lab_rtx_for_bb): Convert from pointer_map_t to
	pointer_map<rtx>.
	(label_rtx_for_bb): Update for conversion of lab_rtx_for_bb to
	a pointer_map<rtx>, eliminating casts from void* to rtx.
	(expand_gimple_basic_block): Likewise.
	(pass_expand::execute): Likewise, using new/delete of
	pointer_map<rtx> rathern than pointer_map_create/destroy.  NULLify
	the lab_rtx_for_bb ptr after deletion for good measure.
---
 gcc/cfgexpand.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Comments

Jeff Law Aug. 12, 2014, 8:50 p.m. UTC | #1
On 08/06/14 11:19, David Malcolm wrote:
> This gives a slight improvement in typesafety in cfgexpand.c
>
> gcc/
> 	* cfgexpand.c (lab_rtx_for_bb): Convert from pointer_map_t to
> 	pointer_map<rtx>.
> 	(label_rtx_for_bb): Update for conversion of lab_rtx_for_bb to
> 	a pointer_map<rtx>, eliminating casts from void* to rtx.
> 	(expand_gimple_basic_block): Likewise.
> 	(pass_expand::execute): Likewise, using new/delete of
> 	pointer_map<rtx> rathern than pointer_map_create/destroy.  NULLify
> 	the lab_rtx_for_bb ptr after deletion for good measure.
OK.    I think this is still appropriate.  It might even still apply 
cleanly.

Seems like this could have gone forward independently of everything else.


jeff
Trevor Saunders Aug. 12, 2014, 9:15 p.m. UTC | #2
On Tue, Aug 12, 2014 at 02:50:39PM -0600, Jeff Law wrote:
> On 08/06/14 11:19, David Malcolm wrote:
> >This gives a slight improvement in typesafety in cfgexpand.c
> >
> >gcc/
> >	* cfgexpand.c (lab_rtx_for_bb): Convert from pointer_map_t to
> >	pointer_map<rtx>.
> >	(label_rtx_for_bb): Update for conversion of lab_rtx_for_bb to
> >	a pointer_map<rtx>, eliminating casts from void* to rtx.
> >	(expand_gimple_basic_block): Likewise.
> >	(pass_expand::execute): Likewise, using new/delete of
> >	pointer_map<rtx> rathern than pointer_map_create/destroy.  NULLify
> >	the lab_rtx_for_bb ptr after deletion for good measure.
> OK.    I think this is still appropriate.  It might even still apply
> cleanly.

actually I suspect this patch is totally obsolete after my patches last
week to remove pointer_map. This is now a hash_map<basic_block, rtx> *.

sorry about the duplicated effort :/


Trev

> 
> Seems like this could have gone forward independently of everything else.
> 
> 
> jeff
> 
>
David Malcolm Aug. 13, 2014, 12:45 a.m. UTC | #3
On Tue, 2014-08-12 at 17:15 -0400, Trevor Saunders wrote:
> On Tue, Aug 12, 2014 at 02:50:39PM -0600, Jeff Law wrote:
> > On 08/06/14 11:19, David Malcolm wrote:
> > >This gives a slight improvement in typesafety in cfgexpand.c
> > >
> > >gcc/
> > >	* cfgexpand.c (lab_rtx_for_bb): Convert from pointer_map_t to
> > >	pointer_map<rtx>.
> > >	(label_rtx_for_bb): Update for conversion of lab_rtx_for_bb to
> > >	a pointer_map<rtx>, eliminating casts from void* to rtx.
> > >	(expand_gimple_basic_block): Likewise.
> > >	(pass_expand::execute): Likewise, using new/delete of
> > >	pointer_map<rtx> rathern than pointer_map_create/destroy.  NULLify
> > >	the lab_rtx_for_bb ptr after deletion for good measure.
> > OK.    I think this is still appropriate.  It might even still apply
> > cleanly.
> 
> actually I suspect this patch is totally obsolete after my patches last
> week to remove pointer_map. This is now a hash_map<basic_block, rtx> *.
> 
> sorry about the duplicated effort :/

No worries.

I believe in an earlier version of this patchkit I then updated it from
pointer_map<rtx> to pointer_map<rtx_code_label *>.

In theory the fix would then be to convert it from
  hash_map<basic_block, rtx> *
to
  hash_map<basic_block, rtx_code_label *> *

But looking over the patches it looks like I dropped the later usage of
rtx_code_label * for some reason (perhaps when I ran into the issues
mentioned in patch 2).

Maybe something to look at once the rest of the patches are in, I guess.

> Trev
> 
> > 
> > Seems like this could have gone forward independently of everything else.
> > 
> > 
> > jeff
> > 
> >
Jeff Law Aug. 13, 2014, 2:50 a.m. UTC | #4
On 08/12/14 18:45, David Malcolm wrote:
> On Tue, 2014-08-12 at 17:15 -0400, Trevor Saunders wrote:
>> On Tue, Aug 12, 2014 at 02:50:39PM -0600, Jeff Law wrote:
>>> On 08/06/14 11:19, David Malcolm wrote:
>>>> This gives a slight improvement in typesafety in cfgexpand.c
>>>>
>>>> gcc/
>>>> 	* cfgexpand.c (lab_rtx_for_bb): Convert from pointer_map_t to
>>>> 	pointer_map<rtx>.
>>>> 	(label_rtx_for_bb): Update for conversion of lab_rtx_for_bb to
>>>> 	a pointer_map<rtx>, eliminating casts from void* to rtx.
>>>> 	(expand_gimple_basic_block): Likewise.
>>>> 	(pass_expand::execute): Likewise, using new/delete of
>>>> 	pointer_map<rtx> rathern than pointer_map_create/destroy.  NULLify
>>>> 	the lab_rtx_for_bb ptr after deletion for good measure.
>>> OK.    I think this is still appropriate.  It might even still apply
>>> cleanly.
>>
>> actually I suspect this patch is totally obsolete after my patches last
>> week to remove pointer_map. This is now a hash_map<basic_block, rtx> *.
I haven't updated my tree recently, so I don't have those in my tree yet :-)


Jeff
diff mbox

Patch

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 934f40d..d124d94 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1956,7 +1956,7 @@  maybe_dump_rtl_for_gimple_stmt (gimple stmt, rtx since)
 
 /* Maps the blocks that do not contain tree labels to rtx labels.  */
 
-static struct pointer_map_t *lab_rtx_for_bb;
+static struct pointer_map<rtx> *lab_rtx_for_bb;
 
 /* Returns the label_rtx expression for a label starting basic block BB.  */
 
@@ -1966,14 +1966,14 @@  label_rtx_for_bb (basic_block bb ATTRIBUTE_UNUSED)
   gimple_stmt_iterator gsi;
   tree lab;
   gimple lab_stmt;
-  void **elt;
+  rtx *elt;
 
   if (bb->flags & BB_RTL)
     return block_label (bb);
 
-  elt = pointer_map_contains (lab_rtx_for_bb, bb);
+  elt = lab_rtx_for_bb->contains (bb);
   if (elt)
-    return (rtx) *elt;
+    return *elt;
 
   /* Find the tree label if it is present.  */
 
@@ -1990,9 +1990,9 @@  label_rtx_for_bb (basic_block bb ATTRIBUTE_UNUSED)
       return label_rtx (lab);
     }
 
-  elt = pointer_map_insert (lab_rtx_for_bb, bb);
+  elt = lab_rtx_for_bb->insert (bb);
   *elt = gen_label_rtx ();
-  return (rtx) *elt;
+  return *elt;
 }
 
 
@@ -4880,7 +4880,7 @@  expand_gimple_basic_block (basic_block bb, bool disable_tail_calls)
   rtx note, last;
   edge e;
   edge_iterator ei;
-  void **elt;
+  rtx *elt;
 
   if (dump_file)
     fprintf (dump_file, "\n;; Generating RTL for gimple basic block %d\n",
@@ -4924,7 +4924,7 @@  expand_gimple_basic_block (basic_block bb, bool disable_tail_calls)
 	stmt = NULL;
     }
 
-  elt = pointer_map_contains (lab_rtx_for_bb, bb);
+  elt = lab_rtx_for_bb->contains (bb);
 
   if (stmt || elt)
     {
@@ -4937,7 +4937,7 @@  expand_gimple_basic_block (basic_block bb, bool disable_tail_calls)
 	}
 
       if (elt)
-	emit_label ((rtx) *elt);
+	emit_label (*elt);
 
       /* Java emits line number notes in the top of labels.
 	 ??? Make this go away once line number notes are obsoleted.  */
@@ -5797,7 +5797,7 @@  pass_expand::execute (function *fun)
   FOR_EACH_EDGE (e, ei, ENTRY_BLOCK_PTR_FOR_FN (fun)->succs)
     e->flags &= ~EDGE_EXECUTABLE;
 
-  lab_rtx_for_bb = pointer_map_create ();
+  lab_rtx_for_bb = new pointer_map <rtx>;
   FOR_BB_BETWEEN (bb, init_block->next_bb, EXIT_BLOCK_PTR_FOR_FN (fun),
 		  next_bb)
     bb = expand_gimple_basic_block (bb, var_ret_seq != NULL_RTX);
@@ -5822,7 +5822,8 @@  pass_expand::execute (function *fun)
 
   /* Expansion is used by optimization passes too, set maybe_hot_insn_p
      conservatively to true until they are all profile aware.  */
-  pointer_map_destroy (lab_rtx_for_bb);
+  delete lab_rtx_for_bb;
+  lab_rtx_for_bb = NULL;
   free_histograms ();
 
   construct_exit_block ();