This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: fold_const.c/tree_swap_operands_p change from Oct 11 breaks m68k
On Sat, Oct 25, 2003 at 01:21:57PM -0700, Richard Henderson wrote:
> On Sat, Oct 25, 2003 at 01:35:54PM -0600, Roger Sayle wrote:
> > (insn 49 38 39 0 (set (reg/s:SI %a0 [32])
> > (reg:SI %d0 [33])) 25 {*m68k.md:760} (insn_list 38 (nil))
> > (nil))
> >
> > (insn 39 49 40 0 (set (cc0)
> > (reg:SI %d0 [33])) 3 {*m68k.md:196} (nil)
> > (expr_list:REG_DEAD (reg:SI %d0 [33])
> > (nil)))
> [...]
> > I'm not really a back-end expert. Here is the relevant define_insn
> > from line 196 of m68k.md. Can anyone see an obvious mistake?
>
> Let me introduce you to NOTICE_UPDATE_CC.
>
> Incidentally, it's not your bug. Indeed, your patch let us notice
> that %d1 was zero, and compare against zero directly instead of
> loading that into a register.
Since %d1 must be initialized with zero anyway, its perfectly fine to
use %d1 in the compare. I don't think that using tstl is any better.
I noticed before, that GCC at times believes that directly using a
value is better than loading it in a register before.
On Sun, Oct 26, 2003 at 01:50:43AM -0800, Richard Henderson wrote:
> Index: m68k.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/config/m68k/m68k.c,v
> retrieving revision 1.114
> diff -u -p -r1.114 m68k.c
> --- m68k.c 22 Oct 2003 21:27:27 -0000 1.114
> +++ m68k.c 26 Oct 2003 09:49:43 -0000
> @@ -2605,12 +2605,17 @@ notice_update_cc (rtx exp, rtx insn)
> if (cc_status.value2 != 0)
> switch (GET_CODE (cc_status.value2))
> {
> - case PLUS: case MINUS: case MULT:
> - case DIV: case UDIV: case MOD: case UMOD: case NEG:
> -#if 0 /* These instructions always clear the overflow bit */
> case ASHIFT: case ASHIFTRT: case LSHIFTRT:
> case ROTATE: case ROTATERT:
> -#endif
> + /* These instructions always clear the overflow bit, and set
> + the carry to the bit shifted out. */
> + /* ??? We don't currently have a way to signal carry not valid,
> + nor do we check for it in the branch insns. */
> + CC_STATUS_INIT;
> + break;
> +
> + case PLUS: case MINUS: case MULT:
> + case DIV: case UDIV: case MOD: case UMOD: case NEG:
> if (GET_MODE (cc_status.value2) != VOIDmode)
> cc_status.flags |= CC_NO_OVERFLOW;
> break;
This patch fixes the breakage. Now GCC emits a tst.l instruction that
correctly sets the condition codes:
_foo: moveq #0,d1
movel sp@(4),d0
lsrl #5,d0
movel d0,a0
tstl d0
jls L7
L5: addql #1,d1
cmpl a0,d1
jcs L5
L7: rts
Thank you for looking into the problem!
However, now there is another mysterious behaviour with that test code:
void foo (unsigned int j) {
unsigned int i;
for (i = VALUE; i < (j/32); ++i) ;
}
With VALUE between 1 and 127 (both included in the range) GCC saves %d2
(which is not used in the code!):
_foo: movel d2,sp@-
moveq #127,d1
movel sp@(8),d0
lsrl #5,d0
movel d0,a0
cmpl d0,d1
jcc L7
L5: addql #1,d1
cmpl a0,d1
jcs L5
L7: movel sp@+,d2
rts
With VALUE >= 128 that doesn't happen and a 20031008 doesn't save the
register either. I would like to know where I should look to findout
why GCC saves %d2?
Gunther