This patch addresses an issue Nicolai found while browsing combine.c.
He noticed some checking code that looked like:
for (i = 0; i < NUM_ARGS; i++)
if (!cond)
break;
for (i = 0; i < NUM_ARGS; i++)
if (!other_cond)
break;
if (i == NUM_ARGS)
do_the_thing;
the intent is that both cond and other_cond are true for all args, but
the first loop's negative result (i != NUM_ARGS) is being ignored. This
patch fixes that problem, and I took the opportunity to merge both loops
into a single loop:
ok = true;
for (i = 0; ok && i < NUM_ARGS; i++)
if (!cond || !other_cond)
ok = false;
if (ok)
do_the_thing;
Segher commented on IRC that a single loop would be slower. I disagree.
These days the optimizer is smart enough to turn that boolean logic
into direct control flow, and a single loop is certainly smaller code (I
checked). The only case that might be faster would be an early out if
the first loop resulted in a negative. But I doubt that's significant,
particularly as the first loop's check is the more expensive one and the
second loops check is very cheap.
booted & tested on x86_64-linux. Thanks to Nic for finding this problem
and helping test it.
nathan
--
Nathan Sidwell
2017-01-10 Nathan Sidwell <nat...@acm.org>
Nicolai Stange <nicsta...@gmail.com>
* combine.c (try_combine): Don't ignore result of overlap checking
loop. Combine overlap & asm check into single loop.
Index: combine.c
===================================================================
--- combine.c (revision 244226)
+++ combine.c (working copy)
@@ -2785,22 +2785,24 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
(Besides, reload can't handle output reloads for this.)
The problem can also happen if the dest of I3 is a memory ref,
- if another dest in I2 is an indirect memory ref. */
- for (i = 0; i < XVECLEN (p2, 0); i++)
- if ((GET_CODE (XVECEXP (p2, 0, i)) == SET
- || GET_CODE (XVECEXP (p2, 0, i)) == CLOBBER)
- && reg_overlap_mentioned_p (SET_DEST (PATTERN (i3)),
- SET_DEST (XVECEXP (p2, 0, i))))
- break;
+ if another dest in I2 is an indirect memory ref.
- /* Make sure this PARALLEL is not an asm. We do not allow combining
+ Neither can this PARALLEL be not an asm. We do not allow combining
that usually (see can_combine_p), so do not here either. */
- for (i = 0; i < XVECLEN (p2, 0); i++)
- if (GET_CODE (XVECEXP (p2, 0, i)) == SET
- && GET_CODE (SET_SRC (XVECEXP (p2, 0, i))) == ASM_OPERANDS)
- break;
+ bool ok = true;
+ for (i = 0; ok && i < XVECLEN (p2, 0); i++)
+ {
+ if ((GET_CODE (XVECEXP (p2, 0, i)) == SET
+ || GET_CODE (XVECEXP (p2, 0, i)) == CLOBBER)
+ && reg_overlap_mentioned_p (SET_DEST (PATTERN (i3)),
+ SET_DEST (XVECEXP (p2, 0, i))))
+ ok = false;
+ else if (GET_CODE (XVECEXP (p2, 0, i)) == SET
+ && GET_CODE (SET_SRC (XVECEXP (p2, 0, i))) == ASM_OPERANDS)
+ ok = false;
+ }
- if (i == XVECLEN (p2, 0))
+ if (ok)
for (i = 0; i < XVECLEN (p2, 0); i++)
if (GET_CODE (XVECEXP (p2, 0, i)) == SET
&& SET_DEST (XVECEXP (p2, 0, i)) == SET_SRC (PATTERN (i3)))
