This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
PATCH RFA: PR gcc/1532: jump to following instruction
- From: Ian Lance Taylor <ian at wasabisystems dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: 25 Jan 2004 09:39:45 -0500
- Subject: 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