Re: [rz@linux-m68k.org: Re: m68k, extendqidi2 problem]

[Gunther Nikl <gni at gecko dot de>]

On Wed, Mar 17, 2004 at 11:11:31PM +0100, Richard Zidlicky wrote:
> On Wed, Mar 17, 2004 at 01:17:32PM +0100, Gunther Nikl wrote:
> > > +      if (TARGET_68020 || TARGET_COLDFIRE)
> > > +        return "move%.w %1,%2\;extb%.l %2\;smi %0\;extb%.l %0";
> > > +      else
> > > +        return "move%.w %1,%2\;ext%.w %0\;ext%.l %2\;move%.l %2,%0\;smi %0";
> > 
> >   Why "ext%.w %0"? Shouldn't it be "ext%.w %2"? Then I don't understand
> >   the "move%.l %2,%0 in the else case and the extbl after smi in the if
> >   case.
> 
> extb.l takes a byte and extends to long. ext.w extends byte to 16 bit.
> It would work but needs an extra insn

  I know. I was talking about movel+smi and smi+extbl.

> > > +   return "move%.w %1,%2\;ext%.w %0\;ext%.l %2\;move%.l %2,%0\;smi %0";
>                                 ^^^^^^^^^
> Thinking more about your question.. does that look like yet another bug
> in the same pattern ?! 
> 
> So lets assume
> 
> > > +   return "move%.w %1,%2\;ext%.w %2\;ext%.l %2\;move%.l %2,%0\;smi %0";
> 
> ..  makes more sense to me. The "move.l %2,%0" will set or clear upper 24
> bits, smi the lower 8 bits of the high word register. Pure curiosity, why
> isn't the same trick used in the 68020 branch?

  I would use it also in the 68020 case. Then the only difference between
  the two cases is extw+extl vs. extbl. I guess movel is faster than extbl
  but maybe smi+extbl is better for pipelining. In that case your new
  version would be fine.

> Has that pattern ever been used before? Any more bugs in it?
> I have now tried to convert it to "%R" notation.. 
> 
> (define_insn "extendqidi2"
>   [(set (match_operand:DI 0 "nonimmediate_operand" "=d")
>         (sign_extend:DI (match_operand:QI 1 "general_src_operand" "rmS")))]
>   ""
> {
>   CC_STATUS_INIT;
>   if (ADDRESS_REG_P(operands[1]))
>     {
>       if (TARGET_68020 || TARGET_COLDFIRE)
>         return "move%.w %1,%R0\;extb%.l %R0\;smi %0\;extb%.l %0";
>       else
>         return "move%.w %1,%R0\;ext%.w %R0\;ext%.l %R0\;move%.l %R0,%0\;smi %0";
>     }
>   else
>     {
>       if (TARGET_68020 || TARGET_COLDFIRE)
>         return "move%.b %1,%R0\;extb%.l %R0\;smi %0\;extb%.l %0";
>       else
>         return "move%.b %1,%R0\;ext%.w %R0\;ext%.l %R0\;move%.l %R0,%0\;smi %0";
>     }
> })

  Gunther