This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PATCH RFA: PR gcc/1532: jump to following instruction


For the test case in PR gcc/1532, mainline gcc currently generates
code that includes this fragment (using -O2);

	cmpl	$1, %eax
	je	.L1
	jle	.L15
	cmpl	$2, %eax
	movl	%ecx, %edx
	je	.L1
.L2:
.L1:
	popl	%ebx

The jump to .L1 is obviously useless, since it jumps to the
immediately following instruction.

This is caused by the following sequence of events:

1) In clobber_return_register(), gcc puts a CLOBBER of the return
   register at the end of the function.  This clobber ensures that the
   return register is not viewed as live as the end of the function,
   in the case where the function drop through without calling return.
   (If the function does call return, this CLOBBER will be discarded
   as unreachable.)

2) In order to make that work, flow_active_insn_p() explicitly checks
   for that CLOBBER, and does not permit skipping over the block which
   contain it.

3) The jump in the generated code shown above is actually jumping over
   the CLOBBER.  Of course the CLOBBER is not inserted into the
   instruction stream, so the result is the useless jump.

Here is the actual code for this test case:

inline int check(int i,int j) {
  if (i==j) return 0;
  else if (i>j) return 1;
  else return 2;
}

int foo(int i,int j) {
  switch (check(i,j)) {
  case 0: return i+j;
  case 1: return i;
  case 2: return j;
  }
}

Looking at this code it is obvious that the function does in fact
always return a value.  It does not drop through.  However, gcc, at
least on mainline, is not quite clever enough to see that.

With a small patch after the reload pass is complete, we can detect
that the CLOBBER is useless here.  This is because after reload we can
confirm that, in this case, the return register is actually always set
to a value.  That means that we can safely discard the CLOBBER, since
the return register is effectively clobbered anyhow.

With the appended patch, I see this in the generated code for the test
case.

	cmpl	$1, %eax
	je	.L1
	jle	.L15
	movl	%ecx, %edx
.L1:
	popl	%ebx

In other words, the useless jump was removed, and that caused the
compare which triggered the jump to become useless and to also be
removed.

The patch should have minimal performance impact, since it just adds a
couple more tests to code which already walks through the entire
instruction list.  I am currently running the testsuite and a
bootstrap on i686-pc-linux-gnu.  I'm confident that they will pass,
since a slightly different version of the patch already passed.

OK for mainline if all the tests pass?

Ian


2004-01-25  Ian Lance Taylor  <ian@wasabisystems.com>

	PR gcc/1532
	* reload1.c (reload): We can delete the CLOBBER of the return
	register if it is followed by an instruction which modifies the
	return register.


Index: reload1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload1.c,v
retrieving revision 1.420
diff -p -u -r1.420 reload1.c
--- reload1.c	17 Jan 2004 22:14:17 -0000	1.420
+++ reload1.c	25 Jan 2004 14:09:15 -0000
@@ -632,6 +632,7 @@ reload (rtx first, int global)
   rtx insn;
   struct elim_table *ep;
   basic_block bb;
+  rtx clobber_return_insn;
 
   /* Make sure even insns with volatile mem refs are recognizable.  */
   init_recog ();
@@ -1142,8 +1143,11 @@ reload (rtx first, int global)
      are no longer useful or accurate.  Strip and regenerate REG_INC notes
      that may have been moved around.  */
 
+  clobber_return_insn = NULL_RTX;
   for (insn = first; insn; insn = NEXT_INSN (insn))
-    if (INSN_P (insn))
+    if (GET_CODE (insn) == BARRIER)
+      clobber_return_insn = NULL_RTX;
+    else if (INSN_P (insn))
       {
 	rtx *pnote;
 
@@ -1151,18 +1155,32 @@ reload (rtx first, int global)
 	  replace_pseudos_in (& CALL_INSN_FUNCTION_USAGE (insn),
 			      VOIDmode, CALL_INSN_FUNCTION_USAGE (insn));
 
-	if ((GET_CODE (PATTERN (insn)) == USE
+	if (GET_CODE (PATTERN (insn)) == CLOBBER
+	    && GET_CODE (XEXP (PATTERN (insn), 0)) == REG
+	    && REG_FUNCTION_VALUE_P (XEXP (PATTERN (insn), 0)))
+	  {
+	    /* We can delete the CLOBBER of the return value in the
+	       special case that we find an immediately following
+	       assignment to it, since that can play the part of the
+	       CLOBBER for following passes.  This can help generate
+	       better code, because CFG has a special case for the
+	       CLOBBER of the return value.  Start looking for an
+	       immediately following assignment here.  ??? This
+	       approach doesn't work for functions which return values
+	       in more than one place.  */
+	    clobber_return_insn = insn;
+	  }
+	else if ((GET_CODE (PATTERN (insn)) == USE
 	     /* We mark with QImode USEs introduced by reload itself.  */
-	     && (GET_MODE (insn) == QImode
-		 || find_reg_note (insn, REG_EQUAL, NULL_RTX)))
-	    || (GET_CODE (PATTERN (insn)) == CLOBBER
-		&& (GET_CODE (XEXP (PATTERN (insn), 0)) != MEM
-		    || GET_MODE (XEXP (PATTERN (insn), 0)) != BLKmode
-		    || (GET_CODE (XEXP (XEXP (PATTERN (insn), 0), 0)) != SCRATCH
-			&& XEXP (XEXP (PATTERN (insn), 0), 0)
-				!= stack_pointer_rtx))
-		&& (GET_CODE (XEXP (PATTERN (insn), 0)) != REG
-		    || ! REG_FUNCTION_VALUE_P (XEXP (PATTERN (insn), 0)))))
+		  && (GET_MODE (insn) == QImode
+		      || find_reg_note (insn, REG_EQUAL, NULL_RTX)))
+		 || (GET_CODE (PATTERN (insn)) == CLOBBER
+		     && (GET_CODE (XEXP (PATTERN (insn), 0)) != MEM
+			 || GET_MODE (XEXP (PATTERN (insn), 0)) != BLKmode
+			 || ((GET_CODE (XEXP (XEXP (PATTERN (insn), 0), 0))
+			      != SCRATCH)
+			     && (XEXP (XEXP (PATTERN (insn), 0), 0)
+				 != stack_pointer_rtx)))))
 	  {
 	    delete_insn (insn);
 	    continue;
@@ -1194,6 +1212,27 @@ reload (rtx first, int global)
 
 	/* And simplify (subreg (reg)) if it appears as an operand.  */
 	cleanup_subreg_operands (insn);
+
+	if (clobber_return_insn)
+	  {
+	    if (GET_CODE (insn) == CALL_INSN
+		|| GET_CODE (insn) == JUMP_INSN)
+	      clobber_return_insn = NULL_RTX;
+	    else if (GET_CODE (PATTERN (insn)) != CLOBBER
+		     && reg_set_p (XEXP (PATTERN (clobber_return_insn), 0),
+				   insn)
+		     && ! noop_move_p (insn))
+	      {
+		/* We found an assignment to the return value
+		   immediately following the clobber, so we can delete
+		   the clobber.  We have to explicitly check
+		   noop_move_p in the condition, because such a move
+		   may get deleted elsewhere, in which case the
+		   clobber will be needed.  */
+		delete_insn (clobber_return_insn);
+		clobber_return_insn = NULL_RTX;
+	      }
+	  }
       }
 
   /* If we are doing stack checking, give a warning if this function's


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]