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]

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]);
> 



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