This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix *two* AVR backend bugs (PR19293 + PR19329)
- From: Paul Schlie <schlie at comcast dot net>
- To: Marek Michalkiewicz <marekm at amelek dot gda dot pl>
- Cc: Bernardo Innocenti <bernie at develer dot com>,Giovanni Bajo <rasky at develer dot com>,<denisc at overta dot ru>,Stefano Fedrigo <aleph at develer dot com>,<gcc-patches at gcc dot gnu dot org>,Mark Mitchell <mark at codesourcery dot com>
- Date: Sun, 23 Jan 2005 19:52:15 -0500
- Subject: Re: [PATCH] Fix *two* AVR backend bugs (PR19293 + PR19329)
Hi, Using a 4.0 cvs sources about a week old; it appears that variable left
shift code seem to have been superceded by a sequence of adds (apparently a
result of middle-end optimizations assuming adds are cheaper than shifts?)
With the exception of the above, or what it's worth: the optimization
setting the n'th bit in _zero_reg_ then shifting it, has the effect of
producing a modulo 8 shift, not simply confusing a -128 with 128, etc.;
Again, as above, nor is it not clear that if GCC generates a 0 shift, if
it presumes that the condition codes have been set as defined by avr.md,
therefore a no-op may possibly not be appropriate as the rtl is presently
defined.
> From: Marek Michalkiewicz <marekm@amelek.gda.pl>
>
> Hello,
>
> On Sun, Jan 23, 2005 at 12:38:14PM -0500, Paul Schlie wrote:
>
>> Although acknowledge that a saturating behavior for non-negative shifts may
>> be desirable if the middle-end presumes ((x >> y) >> y) :: (x >> (2 * y)),
>> or it's analogy for left-shifts; but it's not clear that it officially does?
>
> Please look into the patch below, test it a little to be sure, and tell
> me if it's OK to commit. Now the undefined behaviour of shifts with
> out of range constant/variable counts should be consistent (almost...
> because non-const -128 becomes +128, but small numbers should work).
>
> Hope this helps,
> Marek
>
> PR target/19293
> PR target/19329
> * config/avr/avr.c (notice_update_cc): Only set condition code for
> ashrqi3 if shift count > 0.
> (out_shift_with_cnt): Handle shift count <= 0 as a no-op.
> (ashlqi3_out, ashlhi3_out, ashlsi3_out, ashrqi3_out, ashrhi3_out,
> ashrsi3_out, lshrqi3_out, lshrhi3_out, lshrsi3_out): Handle shift
> count <= 0 as a no-op, and shift count >= width by copying zero
> or sign bit to all bits of the result.
>
> Index: avr.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/config/avr/avr.c,v
> retrieving revision 1.126
> diff -c -3 -p -r1.126 avr.c
> *** avr.c 23 Jan 2005 04:51:28 -0000 1.126
> --- avr.c 23 Jan 2005 18:17:04 -0000
> *************** notice_update_cc (rtx body ATTRIBUTE_UNU
> *** 1229,1234 ****
> --- 1229,1235 ----
> rtx x = XEXP (src, 1);
>
> if (GET_CODE (x) == CONST_INT
> + && INTVAL (x) > 0
> && INTVAL (x) != 6)
> {
> cc_status.value1 = SET_DEST (set);
> *************** out_shift_with_cnt (const char *template
> *** 2749,2754 ****
> --- 2750,2762 ----
> int count = INTVAL (operands[2]);
> int max_len = 10; /* If larger than this, always use a loop. */
>
> + if (count <= 0)
> + {
> + if (len)
> + *len = 0;
> + return;
> + }
> +
> if (count < 8 && !scratch)
> use_zero_reg = 1;
>
> *************** ashlqi3_out (rtx insn, rtx operands[], i
> *** 2871,2876 ****
> --- 2879,2887 ----
> switch (INTVAL (operands[2]))
> {
> default:
> + if (INTVAL (operands[2]) < 8)
> + break;
> +
> *len = 1;
> return AS1 (clr,%0);
>
> *************** ashlhi3_out (rtx insn, rtx operands[], i
> *** 2967,2972 ****
> --- 2978,2991 ----
>
> switch (INTVAL (operands[2]))
> {
> + default:
> + if (INTVAL (operands[2]) < 16)
> + break;
> +
> + *len = 2;
> + return (AS1 (clr,%B0) CR_TAB
> + AS1 (clr,%A0));
> +
> case 4:
> if (optimize_size && scratch)
> break; /* 5 */
> *************** ashlsi3_out (rtx insn, rtx operands[], i
> *** 3218,3223 ****
> --- 3237,3256 ----
>
> switch (INTVAL (operands[2]))
> {
> + default:
> + if (INTVAL (operands[2]) < 32)
> + break;
> +
> + if (AVR_ENHANCED)
> + return *len = 3, (AS1 (clr,%D0) CR_TAB
> + AS1 (clr,%C0) CR_TAB
> + AS2 (movw,%A0,%C0));
> + *len = 4;
> + return (AS1 (clr,%D0) CR_TAB
> + AS1 (clr,%C0) CR_TAB
> + AS1 (clr,%B0) CR_TAB
> + AS1 (clr,%A0));
> +
> case 8:
> {
> int reg0 = true_regnum (operands[0]);
> *************** ashrqi3_out (rtx insn, rtx operands[], i
> *** 3356,3361 ****
> --- 3389,3399 ----
> AS2 (bld,%0,0));
>
> default:
> + if (INTVAL (operands[2]) < 8)
> + break;
> +
> + /* fall through */
> +
> case 7:
> *len = 2;
> return (AS1 (lsl,%0) CR_TAB
> *************** ashrhi3_out (rtx insn, rtx operands[], i
> *** 3519,3524 ****
> --- 3557,3568 ----
> AS2 (mov,%B0,%A0) CR_TAB
> AS1 (rol,%A0));
>
> + default:
> + if (INTVAL (operands[2]) < 16)
> + break;
> +
> + /* fall through */
> +
> case 15:
> return *len = 3, (AS1 (lsl,%B0) CR_TAB
> AS2 (sbc,%A0,%A0) CR_TAB
> *************** ashrsi3_out (rtx insn, rtx operands[], i
> *** 3626,3631 ****
> --- 3670,3681 ----
> AS2 (mov,%B0,%D0) CR_TAB
> AS2 (mov,%C0,%D0));
>
> + default:
> + if (INTVAL (operands[2]) < 32)
> + break;
> +
> + /* fall through */
> +
> case 31:
> if (AVR_ENHANCED)
> return *len = 4, (AS1 (lsl,%D0) CR_TAB
> *************** lshrqi3_out (rtx insn, rtx operands[], i
> *** 3664,3669 ****
> --- 3714,3722 ----
> switch (INTVAL (operands[2]))
> {
> default:
> + if (INTVAL (operands[2]) < 8)
> + break;
> +
> *len = 1;
> return AS1 (clr,%0);
>
> *************** lshrhi3_out (rtx insn, rtx operands[], i
> *** 3758,3763 ****
> --- 3811,3824 ----
>
> switch (INTVAL (operands[2]))
> {
> + default:
> + if (INTVAL (operands[2]) < 16)
> + break;
> +
> + *len = 2;
> + return (AS1 (clr,%B0) CR_TAB
> + AS1 (clr,%A0));
> +
> case 4:
> if (optimize_size && scratch)
> break; /* 5 */
> *************** lshrsi3_out (rtx insn, rtx operands[], i
> *** 4008,4013 ****
> --- 4069,4088 ----
>
> switch (INTVAL (operands[2]))
> {
> + default:
> + if (INTVAL (operands[2]) < 32)
> + break;
> +
> + if (AVR_ENHANCED)
> + return *len = 3, (AS1 (clr,%D0) CR_TAB
> + AS1 (clr,%C0) CR_TAB
> + AS2 (movw,%A0,%C0));
> + *len = 4;
> + return (AS1 (clr,%D0) CR_TAB
> + AS1 (clr,%C0) CR_TAB
> + AS1 (clr,%B0) CR_TAB
> + AS1 (clr,%A0));
> +
> case 8:
> {
> int reg0 = true_regnum (operands[0]);
>