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

[Richard Zidlicky <rz@linux-m68k.org>]

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

> > +   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?

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";
    }
})



Richard