diff mbox

[2/2] refactor emit_*_{after,before}{,_setloc} using common functions

Message ID 20110323201744.GA18264@nightcrawler
State New
Headers show

Commit Message

Nathan Froyd March 23, 2011, 8:17 p.m. UTC 
On Wed, Mar 23, 2011 at 11:36:26AM -0700, Richard Henderson wrote:
> On 03/23/2011 05:09 AM, Nathan Froyd wrote:
> > Did you mean loc == UNKNOWN_LOCATION?  Also, it looks like that's
> > conflating INSN_LOCATORs and location_ts; it seems like it'd be better
> > to keep them separate.
> 
> Ug.  Yes and yes.  I'd forgotten that insn_locators are different
> from line locations.  And indeed, there's no magic define for the
> no-insn-locator condition; it's just a literal zero.
> 
> But still, there's no reason not to apply the same zero optimization
> for all insn types.

Zero optimization implemented, !loc test retained.  Bootstrapped OK; OK
to commit?

-Nathan

	* emit-rtl.c (emit_pattern_after_setloc): New function.
	(emit_insn_after_setloc, emit_jump_insn_after_setloc): Call it.
	(emit_call_insn_after_setloc, emit_debug_insn_after_setloc): Likewise.
	(emit_pattern_after): New function.
	(emit_insn_after, emit_jump_insn_after): Call it.
	(emit_call_insn_after, emit_debug_insn_after): Likewise.
	(emit_pattern_before_setloc): New function.
	(emit_insn_before_setloc, emit_jump_insn_before_setloc): Call it.
	(emit_call_insn_before_setloc, emit_debug_insn_before_setloc):
	Likewise.
	(emit_pattern_before): New function.
	(emit_insn_before, emit_jump_insn_before): Call it.
	(emit_call_insn_before, emit_debug_insn_before): Likewise.

Comments

Richard Henderson March 23, 2011, 8:55 p.m. UTC | #1 
On 03/23/2011 01:17 PM, Nathan Froyd wrote:
> 	* emit-rtl.c (emit_pattern_after_setloc): New function.
> 	(emit_insn_after_setloc, emit_jump_insn_after_setloc): Call it.
> 	(emit_call_insn_after_setloc, emit_debug_insn_after_setloc): Likewise.
> 	(emit_pattern_after): New function.
> 	(emit_insn_after, emit_jump_insn_after): Call it.
> 	(emit_call_insn_after, emit_debug_insn_after): Likewise.
> 	(emit_pattern_before_setloc): New function.
> 	(emit_insn_before_setloc, emit_jump_insn_before_setloc): Call it.
> 	(emit_call_insn_before_setloc, emit_debug_insn_before_setloc):
> 	Likewise.
> 	(emit_pattern_before): New function.
> 	(emit_insn_before, emit_jump_insn_before): Call it.
> 	(emit_call_insn_before, emit_debug_insn_before): Likewise.

Ok.

Please look into removing the "last" parameter from 
emit_pattern_before_noloc, which would allow removing the "insnp"
parameter from emit_pattern_before_setloc.

I can't see how emitting a NULL_RTX should be a Good Thing, ever,
and thus I don't see why we should be doing special things to 
handle it.


r~
Nathan Froyd March 23, 2011, 9:40 p.m. UTC | #2 
On Wed, Mar 23, 2011 at 01:55:34PM -0700, Richard Henderson wrote:
> I can't see how emitting a NULL_RTX should be a Good Thing, ever,
> and thus I don't see why we should be doing special things to 
> handle it.

Ah, I thought the same thing and gcc_assert'ed emitting a NULL_RTX.  I
didn't get very far; at least reload (and possibly other places) assume
that they can do something like:

    x = NULL_RTX;
    ...do something maybe setting x to a useful pattern...
    ...emit x unconditionally...

I can look into just how many places might need to be fixed up because
of this, but scattering a bunch of ifs all over the place seemed less
elegant than handling it all in the emit* functions.

-Nathan
Richard Henderson March 24, 2011, 5:28 p.m. UTC | #3 
On 03/23/2011 02:40 PM, Nathan Froyd wrote:
> I can look into just how many places might need to be fixed up because
> of this, but scattering a bunch of ifs all over the place seemed less
> elegant than handling it all in the emit* functions.

Yeah, maybe.  On the other hand, the other complication that's going on
there is attempting to provide a "sensible" return value in the case of
emitting a NULL_RTX, and doing different things for insns vs non-insns.

Merely removing the "different things" is surely an improvement.
Getting rid of the "sensible" probably cleans things up more, but 
probably interferes more with reload.



r~
diff mbox

Patch

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 9b6f0f8..a4e8f8a 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -4316,11 +4316,14 @@  emit_note_after (enum insn_note subtype, rtx after)
   return note;
 }
 
-/* Like emit_insn_after_noloc, but set INSN_LOCATOR according to SCOPE.  */
-rtx
-emit_insn_after_setloc (rtx pattern, rtx after, int loc)
+/* Insert PATTERN after AFTER, setting its INSN_LOCATION to LOC.
+   MAKE_RAW indicates how to turn PATTERN into a real insn.  */
+
+static rtx
+emit_pattern_after_setloc (rtx pattern, rtx after, int loc,
+			   rtx (*make_raw) (rtx))
 {
-  rtx last = emit_insn_after_noloc (pattern, after, NULL);
+  rtx last = emit_pattern_after_noloc (pattern, after, NULL, make_raw);
 
   if (pattern == NULL_RTX || !loc)
     return last;
@@ -4337,130 +4340,96 @@  emit_insn_after_setloc (rtx pattern, rtx after, int loc)
   return last;
 }
 
-/* Like emit_insn_after_noloc, but set INSN_LOCATOR according to AFTER.  */
-rtx
-emit_insn_after (rtx pattern, rtx after)
+/* Insert PATTERN after AFTER.  MAKE_RAW indicates how to turn PATTERN
+   into a real insn.  SKIP_DEBUG_INSNS indicates whether to insert after
+   any DEBUG_INSNs.  */
+
+static rtx
+emit_pattern_after (rtx pattern, rtx after, bool skip_debug_insns,
+		    rtx (*make_raw) (rtx))
 {
   rtx prev = after;
 
-  while (DEBUG_INSN_P (prev))
-    prev = PREV_INSN (prev);
+  if (skip_debug_insns)
+    while (DEBUG_INSN_P (prev))
+      prev = PREV_INSN (prev);
 
   if (INSN_P (prev))
-    return emit_insn_after_setloc (pattern, after, INSN_LOCATOR (prev));
+    return emit_pattern_after_setloc (pattern, after, INSN_LOCATOR (prev),
+				      make_raw);
   else
-    return emit_insn_after_noloc (pattern, after, NULL);
+    return emit_pattern_after_noloc (pattern, after, NULL, make_raw);
 }
 
-/* Like emit_jump_insn_after_noloc, but set INSN_LOCATOR according to SCOPE.  */
+/* Like emit_insn_after_noloc, but set INSN_LOCATOR according to LOC.  */
 rtx
-emit_jump_insn_after_setloc (rtx pattern, rtx after, int loc)
+emit_insn_after_setloc (rtx pattern, rtx after, int loc)
 {
-  rtx last = emit_jump_insn_after_noloc (pattern, after);
+  return emit_pattern_after_setloc (pattern, after, loc, make_insn_raw);
+}
 
-  if (pattern == NULL_RTX || !loc)
-    return last;
+/* Like emit_insn_after_noloc, but set INSN_LOCATOR according to AFTER.  */
+rtx
+emit_insn_after (rtx pattern, rtx after)
+{
+  return emit_pattern_after (pattern, after, true, make_insn_raw);
+}
 
-  after = NEXT_INSN (after);
-  while (1)
-    {
-      if (active_insn_p (after) && !INSN_LOCATOR (after))
-	INSN_LOCATOR (after) = loc;
-      if (after == last)
-	break;
-      after = NEXT_INSN (after);
-    }
-  return last;
+/* Like emit_jump_insn_after_noloc, but set INSN_LOCATOR according to LOC.  */
+rtx
+emit_jump_insn_after_setloc (rtx pattern, rtx after, int loc)
+{
+  return emit_pattern_after_setloc (pattern, after, loc, make_jump_insn_raw);
 }
 
 /* Like emit_jump_insn_after_noloc, but set INSN_LOCATOR according to AFTER.  */
 rtx
 emit_jump_insn_after (rtx pattern, rtx after)
 {
-  rtx prev = after;
-
-  while (DEBUG_INSN_P (prev))
-    prev = PREV_INSN (prev);
-
-  if (INSN_P (prev))
-    return emit_jump_insn_after_setloc (pattern, after, INSN_LOCATOR (prev));
-  else
-    return emit_jump_insn_after_noloc (pattern, after);
+  return emit_pattern_after (pattern, after, true, make_jump_insn_raw);
 }
 
-/* Like emit_call_insn_after_noloc, but set INSN_LOCATOR according to SCOPE.  */
+/* Like emit_call_insn_after_noloc, but set INSN_LOCATOR according to LOC.  */
 rtx
 emit_call_insn_after_setloc (rtx pattern, rtx after, int loc)
 {
-  rtx last = emit_call_insn_after_noloc (pattern, after);
-
-  if (pattern == NULL_RTX || !loc)
-    return last;
-
-  after = NEXT_INSN (after);
-  while (1)
-    {
-      if (active_insn_p (after) && !INSN_LOCATOR (after))
-	INSN_LOCATOR (after) = loc;
-      if (after == last)
-	break;
-      after = NEXT_INSN (after);
-    }
-  return last;
+  return emit_pattern_after_setloc (pattern, after, loc, make_call_insn_raw);
 }
 
 /* Like emit_call_insn_after_noloc, but set INSN_LOCATOR according to AFTER.  */
 rtx
 emit_call_insn_after (rtx pattern, rtx after)
 {
-  rtx prev = after;
-
-  while (DEBUG_INSN_P (prev))
-    prev = PREV_INSN (prev);
-
-  if (INSN_P (prev))
-    return emit_call_insn_after_setloc (pattern, after, INSN_LOCATOR (prev));
-  else
-    return emit_call_insn_after_noloc (pattern, after);
+  return emit_pattern_after (pattern, after, true, make_call_insn_raw);
 }
 
-/* Like emit_debug_insn_after_noloc, but set INSN_LOCATOR according to SCOPE.  */
+/* Like emit_debug_insn_after_noloc, but set INSN_LOCATOR according to LOC.  */
 rtx
 emit_debug_insn_after_setloc (rtx pattern, rtx after, int loc)
 {
-  rtx last = emit_debug_insn_after_noloc (pattern, after);
-
-  if (pattern == NULL_RTX || !loc)
-    return last;
-
-  after = NEXT_INSN (after);
-  while (1)
-    {
-      if (active_insn_p (after) && !INSN_LOCATOR (after))
-	INSN_LOCATOR (after) = loc;
-      if (after == last)
-	break;
-      after = NEXT_INSN (after);
-    }
-  return last;
+  return emit_pattern_after_setloc (pattern, after, loc, make_debug_insn_raw);
 }
 
 /* Like emit_debug_insn_after_noloc, but set INSN_LOCATOR according to AFTER.  */
 rtx
 emit_debug_insn_after (rtx pattern, rtx after)
 {
-  if (INSN_P (after))
-    return emit_debug_insn_after_setloc (pattern, after, INSN_LOCATOR (after));
-  else
-    return emit_debug_insn_after_noloc (pattern, after);
+  return emit_pattern_after (pattern, after, false, make_debug_insn_raw);
 }
 
-/* Like emit_insn_before_noloc, but set INSN_LOCATOR according to SCOPE.  */
-rtx
-emit_insn_before_setloc (rtx pattern, rtx before, int loc)
+/* Insert PATTERN before BEFORE, setting its INSN_LOCATION to LOC.
+   MAKE_RAW indicates how to turn PATTERN into a real insn.  INSNP
+   indicates if PATTERN is meant for an INSN as opposed to a JUMP_INSN,
+   CALL_INSN, etc.  */
+
+static rtx
+emit_pattern_before_setloc (rtx pattern, rtx before, int loc, bool insnp,
+			    rtx (*make_raw) (rtx))
 {
   rtx first = PREV_INSN (before);
-  rtx last = emit_insn_before_noloc (pattern, before, NULL);
+  rtx last = emit_pattern_before_noloc (pattern, before,
+                                        insnp ? before : NULL_RTX,
+                                        NULL, make_raw);
 
   if (pattern == NULL_RTX || !loc)
     return last;
@@ -4480,127 +4449,93 @@  emit_insn_before_setloc (rtx pattern, rtx before, int loc)
   return last;
 }
 
-/* Like emit_insn_before_noloc, but set INSN_LOCATOR according to BEFORE.  */
-rtx
-emit_insn_before (rtx pattern, rtx before)
+/* Insert PATTERN before BEFORE.  MAKE_RAW indicates how to turn PATTERN
+   into a real insn.  SKIP_DEBUG_INSNS indicates whether to insert
+   before any DEBUG_INSNs.  INSNP indicates if PATTERN is meant for an
+   INSN as opposed to a JUMP_INSN, CALL_INSN, etc.  */
+
+static rtx
+emit_pattern_before (rtx pattern, rtx before, bool skip_debug_insns,
+		     bool insnp, rtx (*make_raw) (rtx))
 {
   rtx next = before;
 
-  while (DEBUG_INSN_P (next))
-    next = PREV_INSN (next);
+  if (skip_debug_insns)
+    while (DEBUG_INSN_P (next))
+      next = PREV_INSN (next);
 
   if (INSN_P (next))
-    return emit_insn_before_setloc (pattern, before, INSN_LOCATOR (next));
+    return emit_pattern_before_setloc (pattern, before, INSN_LOCATOR (next),
+				       insnp, make_raw);
   else
-    return emit_insn_before_noloc (pattern, before, NULL);
+    return emit_pattern_before_noloc (pattern, before,
+                                      insnp ? before : NULL_RTX,
+                                      NULL, make_raw);
 }
 
-/* like emit_insn_before_noloc, but set insn_locator according to scope.  */
+/* Like emit_insn_before_noloc, but set INSN_LOCATOR according to LOC.  */
 rtx
-emit_jump_insn_before_setloc (rtx pattern, rtx before, int loc)
+emit_insn_before_setloc (rtx pattern, rtx before, int loc)
 {
-  rtx first = PREV_INSN (before);
-  rtx last = emit_jump_insn_before_noloc (pattern, before);
+  return emit_pattern_before_setloc (pattern, before, loc, true,
+				     make_insn_raw);
+}
 
-  if (pattern == NULL_RTX)
-    return last;
+/* Like emit_insn_before_noloc, but set INSN_LOCATOR according to BEFORE.  */
+rtx
+emit_insn_before (rtx pattern, rtx before)
+{
+  return emit_pattern_before (pattern, before, true, true, make_insn_raw);
+}
 
-  first = NEXT_INSN (first);
-  while (1)
-    {
-      if (active_insn_p (first) && !INSN_LOCATOR (first))
-	INSN_LOCATOR (first) = loc;
-      if (first == last)
-	break;
-      first = NEXT_INSN (first);
-    }
-  return last;
+/* like emit_insn_before_noloc, but set INSN_LOCATOR according to LOC.  */
+rtx
+emit_jump_insn_before_setloc (rtx pattern, rtx before, int loc)
+{
+  return emit_pattern_before_setloc (pattern, before, loc, false,
+				     make_jump_insn_raw);
 }
 
 /* Like emit_jump_insn_before_noloc, but set INSN_LOCATOR according to BEFORE.  */
 rtx
 emit_jump_insn_before (rtx pattern, rtx before)
 {
-  rtx next = before;
-
-  while (DEBUG_INSN_P (next))
-    next = PREV_INSN (next);
-
-  if (INSN_P (next))
-    return emit_jump_insn_before_setloc (pattern, before, INSN_LOCATOR (next));
-  else
-    return emit_jump_insn_before_noloc (pattern, before);
+  return emit_pattern_before (pattern, before, true, false,
+			      make_jump_insn_raw);
 }
 
-/* like emit_insn_before_noloc, but set insn_locator according to scope.  */
+/* Like emit_insn_before_noloc, but set INSN_LOCATOR according to LOC.  */
 rtx
 emit_call_insn_before_setloc (rtx pattern, rtx before, int loc)
 {
-  rtx first = PREV_INSN (before);
-  rtx last = emit_call_insn_before_noloc (pattern, before);
-
-  if (pattern == NULL_RTX)
-    return last;
-
-  first = NEXT_INSN (first);
-  while (1)
-    {
-      if (active_insn_p (first) && !INSN_LOCATOR (first))
-	INSN_LOCATOR (first) = loc;
-      if (first == last)
-	break;
-      first = NEXT_INSN (first);
-    }
-  return last;
+  return emit_pattern_before_setloc (pattern, before, loc, false,
+				     make_call_insn_raw);
 }
 
-/* like emit_call_insn_before_noloc,
-   but set insn_locator according to before.  */
+/* Like emit_call_insn_before_noloc,
+   but set insn_locator according to BEFORE.  */
 rtx
 emit_call_insn_before (rtx pattern, rtx before)
 {
-  rtx next = before;
-
-  while (DEBUG_INSN_P (next))
-    next = PREV_INSN (next);
-
-  if (INSN_P (next))
-    return emit_call_insn_before_setloc (pattern, before, INSN_LOCATOR (next));
-  else
-    return emit_call_insn_before_noloc (pattern, before);
+  return emit_pattern_before (pattern, before, true, false,
+			      make_call_insn_raw);
 }
 
-/* like emit_insn_before_noloc, but set insn_locator according to scope.  */
+/* Like emit_insn_before_noloc, but set INSN_LOCATOR according to LOC.  */
 rtx
 emit_debug_insn_before_setloc (rtx pattern, rtx before, int loc)
 {
-  rtx first = PREV_INSN (before);
-  rtx last = emit_debug_insn_before_noloc (pattern, before);
-
-  if (pattern == NULL_RTX)
-    return last;
-
-  first = NEXT_INSN (first);
-  while (1)
-    {
-      if (active_insn_p (first) && !INSN_LOCATOR (first))
-	INSN_LOCATOR (first) = loc;
-      if (first == last)
-	break;
-      first = NEXT_INSN (first);
-    }
-  return last;
+  return emit_pattern_before_setloc (pattern, before, loc, false,
+				     make_debug_insn_raw);
 }
 
-/* like emit_debug_insn_before_noloc,
-   but set insn_locator according to before.  */
+/* Like emit_debug_insn_before_noloc,
+   but set insn_locator according to BEFORE.  */
 rtx
 emit_debug_insn_before (rtx pattern, rtx before)
 {
-  if (INSN_P (before))
-    return emit_debug_insn_before_setloc (pattern, before, INSN_LOCATOR (before));
-  else
-    return emit_debug_insn_before_noloc (pattern, before);
+  return emit_pattern_before (pattern, before, false, false,
+			      make_debug_insn_raw);
 }
 
 /* Take X and emit it at the end of the doubly-linked