diff mbox

[IRA] save a bitmap check

Message ID 558C195C.3010606@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov June 25, 2015, 3:08 p.m. UTC 
On 06/24/2015 05:54 AM, Zhouyi Zhou wrote:
> In function assign_hard_reg, checking the bit of conflict_a in
> consideration_allocno_bitmap is unneccesary, because when retry_p is
> false, conflicting objects are always inside of the same loop_node
> (this is ensured in function process_bb_node_lives which marks the
> living objects to death near the end of that function).
>
>     
>
>

   Thanks for reporting this.  I believe you are right the bitmap check
is not necessary.  When I wrote the code I keep in my mind other
possibilities which were not implemented and I don't see the code will
be necessary for foreseeable future.

   Of course the effect on compilation time is tiny (it is about 0.05%
reported by valgrind lackey on compilation of combine.c with -O2).
Still it is a bit uncomfortable for me that my code wastes unnecessary
computer cycles.

So I modified a bit your code taking Jeff's proposals into account and committed it into trunk.

The patch was bootstrapped on x86-64.

Committed as rev. 224944.


Your email reminded me that I need to commit another your patch
which I promised to commit when GCC is on stage1.  I am going to do it today.

Comments

Vladimir Makarov June 25, 2015, 3:13 p.m. UTC | #1 
On 06/25/2015 11:08 AM, Vladimir Makarov wrote:
> On 06/24/2015 05:54 AM, Zhouyi Zhou wrote:
>> In function assign_hard_reg, checking the bit of conflict_a in
>> consideration_allocno_bitmap is unneccesary, because when retry_p is
>> false, conflicting objects are always inside of the same loop_node
>> (this is ensured in function process_bb_node_lives which marks the
>> living objects to death near the end of that function).
>>
>>
>>
>
>
>
> Your email reminded me that I need to commit another your patch
> which I promised to commit when GCC is on stage1.  I am going to do it 
> today.
>
>
Sorry, it looks like Jeff already did it in April.  So your previous 
patch is already in the trunk.
Zhouyi Zhou June 25, 2015, 3:52 p.m. UTC | #2 
Thanks Vladimir and Jeff for reviewing and doing performance tests for me.
  I am focusing on IRA in my off hours for years because it is
in the middle of compiler backend which doesn't depend on 
specific programming language nor application binary interface,
and is very delicate. 
  Hope I can be of more beneficial :-)

Cheers
Zhouyi


> -----Original Messages-----
> From: "Vladimir Makarov" <vmakarov@redhat.com>
> Sent Time: Thursday, June 25, 2015
> To: "Zhouyi Zhou" <yizhouzhou@ict.ac.cn>, gcc-patches@gcc.gnu.org
> Cc: 
> Subject: Re: [PATCH IRA] save a bitmap check
> 
> On 06/25/2015 11:08 AM, Vladimir Makarov wrote:
> > On 06/24/2015 05:54 AM, Zhouyi Zhou wrote:
> >> In function assign_hard_reg, checking the bit of conflict_a in
> >> consideration_allocno_bitmap is unneccesary, because when retry_p is
> >> false, conflicting objects are always inside of the same loop_node
> >> (this is ensured in function process_bb_node_lives which marks the
> >> living objects to death near the end of that function).
> >>
> >>
> >>
> >
> >
> >
> > Your email reminded me that I need to commit another your patch
> > which I promised to commit when GCC is on stage1.  I am going to do it 
> > today.
> >
> >
> Sorry, it looks like Jeff already did it in April.  So your previous 
> patch is already in the trunk.
> 
>
diff mbox

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 224943)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@ 
+2015-06-25  Zhouyi Zhou  <yizhouzhou@ict.ac.cn>
+	    Vladimir Makarov  <vmakarov@redhat.com>
+
+	* ira-color.c (assign_hard_reg): Remove unecessary bitmap check.
+	Add assert.
+
  2015-06-25  Richard Biener  <rguenther@suse.de>
  
  	* fold-const.c (fold_binary_loc): Move simplification of
Index: ira-color.c
===================================================================
--- ira-color.c	(revision 224943)
+++ ira-color.c	(working copy)
@@ -1733,15 +1733,22 @@  assign_hard_reg (ira_allocno_t a, bool r
  	  /* Reload can give another class so we need to check all
  	     allocnos.  */
  	  if (!retry_p
-	      && (!bitmap_bit_p (consideration_allocno_bitmap,
-				 ALLOCNO_NUM (conflict_a))
-		  || ((!ALLOCNO_ASSIGNED_P (conflict_a)
-		       || ALLOCNO_HARD_REGNO (conflict_a) < 0)
-		      && !(hard_reg_set_intersect_p
-			   (profitable_hard_regs,
-			    ALLOCNO_COLOR_DATA
-			    (conflict_a)->profitable_hard_regs)))))
-	    continue;
+	      && ((!ALLOCNO_ASSIGNED_P (conflict_a)
+		   || ALLOCNO_HARD_REGNO (conflict_a) < 0)
+		  && !(hard_reg_set_intersect_p
+		       (profitable_hard_regs,
+			ALLOCNO_COLOR_DATA
+			(conflict_a)->profitable_hard_regs))))
+	    {
+	      /* All conflict allocnos are in consideration bitmap
+		 when retry_p is false.  It might change in future and
+		 if it happens the assert will be broken.  It means
+		 the code should be modified for the new
+		 assumptions.  */
+	      ira_assert (bitmap_bit_p (consideration_allocno_bitmap,
+					ALLOCNO_NUM (conflict_a)));
+	      continue;
+	    }
  	  conflict_aclass = ALLOCNO_CLASS (conflict_a);
  	  ira_assert (ira_reg_classes_intersect_p
  		      [aclass][conflict_aclass]);