diff mbox

struct resources: remove unch_memory member

Message ID CABu31nPx80SewpFMbi6uSTFkYyLkOm3on2yFEV_RRj052t9f1Q@mail.gmail.com
State New
Headers show

Commit Message

Steven Bosscher May 10, 2013, 11:22 p.m. UTC 
Hello,

This unch_memory in struct resources is a left-over from
RTX_UNCHANGING_P, but it looks like the change-over to MEM_READONLY_P
was done incorrectly: The resource_conflicts_p code now reports
conflicts for insns reading readonly memory and insns reading "normal"
memory or no memory at all.

Spotted by checking why reorg.c failed to fill some slots that my
sched-deps based delay slot scheduler managed to fill.

Bootstrapped&tested on sparc64-unknown-linux-gnu. OK for trunk?

Ciao!
Steven
* resource.h (struct resources): Remove unch_memory member.
	(CLEAR_RESOURCE): Don't clear unch_memory.
	* resource.c (mark_referenced_resources): Don't set it.
	(mark_set_resources): Likewise.
	(mark_target_live_regs): Don't clear it.
	(init_resource_info): Likewise.
	* reorg.c (resource_conflicts_p): Don't compare it.
	(redundant_insn): Don't set it.

Comments

Richard Biener May 13, 2013, 7:48 a.m. UTC | #1 
On Sat, May 11, 2013 at 1:22 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> Hello,
>
> This unch_memory in struct resources is a left-over from
> RTX_UNCHANGING_P, but it looks like the change-over to MEM_READONLY_P
> was done incorrectly: The resource_conflicts_p code now reports
> conflicts for insns reading readonly memory and insns reading "normal"
> memory or no memory at all.
>
> Spotted by checking why reorg.c failed to fill some slots that my
> sched-deps based delay slot scheduler managed to fill.
>
> Bootstrapped&tested on sparc64-unknown-linux-gnu. OK for trunk?

Ok.

Thanks,
Richard.

> Ciao!
> Steven
Eric Botcazou May 14, 2013, 9:02 a.m. UTC | #2 
> This unch_memory in struct resources is a left-over from
> RTX_UNCHANGING_P, but it looks like the change-over to MEM_READONLY_P
> was done incorrectly: The resource_conflicts_p code now reports
> conflicts for insns reading readonly memory and insns reading "normal"
> memory or no memory at all.

Does it really?  One would need to have a SET of a MEM_READONLY_P MEM and this 
isn't supposed to happen (unlike with RTX_UNCHANGING_P).  That being said, the 
patch is a good cleanup.

> Spotted by checking why reorg.c failed to fill some slots that my
> sched-deps based delay slot scheduler managed to fill.

I've been in the business lately so I'm interested in all things reorg.c. :-)

> Bootstrapped&tested on sparc64-unknown-linux-gnu. OK for trunk?

Fine with me.
diff mbox

Patch

Index: resource.h
===================================================================
--- resource.h	(revision 198737)
+++ resource.h	(working copy)
@@ -25,14 +25,13 @@  along with GCC; see the file COPYING3.  If not see
 
 /* Macro to clear all resources.  */
 #define CLEAR_RESOURCE(RES)	\
- do { (RES)->memory = (RES)->unch_memory = (RES)->volatil = (RES)->cc = 0; \
+ do { (RES)->memory = (RES)->volatil = (RES)->cc = 0; \
       CLEAR_HARD_REG_SET ((RES)->regs); } while (0)
 
 /* The resources used by a given insn.  */
 struct resources
 {
   char memory;		/* Insn sets or needs a memory location.  */
-  char unch_memory;	/* Insn sets or needs an "unchanging" MEM.  */
   char volatil;		/* Insn sets or needs a volatile memory loc.  */
   char cc;		/* Insn sets or needs the condition codes.  */
   HARD_REG_SET regs;	/* Which registers are set or needed.  */
Index: resource.c
===================================================================
--- resource.c	(revision 198737)
+++ resource.c	(working copy)
@@ -240,9 +240,7 @@  mark_referenced_resources (rtx x, struct resources
     case MEM:
       /* If this memory shouldn't change, it really isn't referencing
 	 memory.  */
-      if (MEM_READONLY_P (x))
-	res->unch_memory = 1;
-      else
+      if (! MEM_READONLY_P (x))
 	res->memory = 1;
       res->volatil |= MEM_VOLATILE_P (x);
 
@@ -740,7 +738,6 @@  mark_set_resources (rtx x, struct resources *res,
       if (in_dest)
 	{
 	  res->memory = 1;
-	  res->unch_memory |= MEM_READONLY_P (x);
 	  res->volatil |= MEM_VOLATILE_P (x);
 	}
 
@@ -896,7 +893,7 @@  mark_target_live_regs (rtx insns, rtx target, stru
 
   /* We have to assume memory is needed, but the CC isn't.  */
   res->memory = 1;
-  res->volatil = res->unch_memory = 0;
+  res->volatil = 0;
   res->cc = 0;
 
   /* See if we have computed this value already.  */
@@ -1145,7 +1142,6 @@  init_resource_info (rtx epilogue_insn)
 
   end_of_function_needs.cc = 0;
   end_of_function_needs.memory = 1;
-  end_of_function_needs.unch_memory = 0;
   CLEAR_HARD_REG_SET (end_of_function_needs.regs);
 
   if (frame_pointer_needed)
Index: reorg.c
===================================================================
--- reorg.c	(revision 198737)
+++ reorg.c	(working copy)
@@ -276,7 +276,6 @@  static int
 resource_conflicts_p (struct resources *res1, struct resources *res2)
 {
   if ((res1->cc && res2->cc) || (res1->memory && res2->memory)
-      || (res1->unch_memory && res2->unch_memory)
       || res1->volatil || res2->volatil)
     return 1;
 
@@ -1542,7 +1541,6 @@  redundant_insn (rtx insn, rtx target, rtx delay_li
   /* Insns we pass may not set either NEEDED or SET, so merge them for
      simpler tests.  */
   needed.memory |= set.memory;
-  needed.unch_memory |= set.unch_memory;
   IOR_HARD_REG_SET (needed.regs, set.regs);
 
   /* This insn isn't redundant if it conflicts with an insn that either is