Re: [Fwd: Re: [PATCH] MIPS32 DSP instructions again]

["Chao-ying Fu" <fu at mips dot com>]

> b)
>
> HI/LO (acc0)
> new register (acc1)
> new register (acc2)
> new register (acc3)
>
> I'll assume that these new registers are the same size as the HI + LO
> register size?

Yes, this is right.

Regards,
Chao-ying

----- Original Message ----- 
From: "Eric Christopher" <echristo@redhat.com>
To: "Chao-ying Fu" <fu@mips.com>
Cc: "Thiemo Seufer" <ths@networkno.de>; <dom@mips.com>; <ian@airs.com>;
"Paul Koning" <pkoning@equallogic.com>; <nigel@mips.com>; "Richard
Sandiford" <rsandifo@nildram.co.uk>; <radhika@mips.com>;
<binutils@sourceware.org>
Sent: Monday, June 13, 2005 3:14 PM
Subject: Re: [Fwd: Re: [PATCH] MIPS32 DSP instructions again]


> On Mon, 2005-06-13 at 15:05 -0700, Chao-ying Fu wrote:
> > Hello,
> >
> > 1. I think I put extra tab, so that the table is aligned
> > for all cases.
> >
>
> It didn't look like it in the diff. Make sure.
>
> > 2. Someone complained about that I put some magic number
> > inside my code.  So, I generated the magic number using
> > the existing mask.  I can change it back to use the number
> > (ex: 64) directly.
> >
>
> Right. You're doing foo + 64 < 127, why not just put in something like:
>
> min_range:
> max_range:
>
> and check to make sure that the operand is in the range?
>
> All else fails comment.
>
> > 3. I reuse the 'c' format, so that I don't need to create
> > a new format for the same shift and the same mask and the
> > same immediate.  I can create a new mask and the new shift,
> > if you want.
> >
>
> This is fine. Comment your code. "This mask is used by "foo" and "blah"
> types of instructions"
>
> > 4. Some DSP instructions write or read accumulators which
> > may be $ac0, $ac1, $ac2, $ac3.  Because $ac0 is HI/LO,
> > these DSP instructions need the attritubes (WR_HILO/RD_HILO)
> > to inform the assembler to maintain correct HI/LO dependences.
> > Or, if the dependences of accumulators and HI/LO are not necessary
> > at all, I can remove all WR_a and RD_a.
>
> You really really need to document this. Since the instruction set has
> no released documentation I have to take your word on how this works.
> I'm not happy with this as I've mentioned before.
>
> So, if I've got this correct:
>
> a) The accumulators are:
>
> HI (acc0)
> LO (acc1)
> new register (acc2)
> new register (acc3)
>
> or
>
> b)
>
> HI/LO (acc0)
> new register (acc1)
> new register (acc2)
> new register (acc3)
>
> I'll assume that these new registers are the same size as the HI + LO
> register size?
>
> You need to also comment the patch far more than you have.
>
> -eric
>
>