On Mon, Jul 05, 2010 at 09:33:36AM +0900, Shinya Kuribayashi wrote:
> On 7/4/2010 2:03 AM, Maciej W. Rozycki wrote:
> > Malta also supports a couple of MIPS IV processors too, so please be
> > careful about such assumptions.
>
> Ah, that's the answer I'm looking for, thanks! So current irq_ffs()
> form (clz() is enabled only when CONFIG_CPU_MIPS32/64 is selected) is
> well-suited for Malta platform, and it seems better to leave them as
> they are. I'll drop the patch from my list.
>
> >> + if (__builtin_constant_p(cpu_has_clo_clz) && cpu_has_clo_clz) {
> >> + int x;
> >> + __asm__(
> >> + " .set push \n"
> >> + " .set mips32 \n"
> >> + " clz %0, %1 \n"
> >> + " .set pop \n"
> >> + : "=r" (x)
> >> + : "r" (pending));
> >> +
> >> + return -x + 31 - CAUSEB_IP;
> >> + }
> >
> > Hmm, ".set mips32" looks dodgy here. For pre-MIPS32/64 platforms this
> > code should never make it to the assembler and if it did, then a
> > build-time error is better than a run-time problem.
For pedantic accuracy - the IDT RC32364 introduced CLO and CLZ; in an act
of uglyness the RC64574 then inherited these two instructions but did not
add. though it was 64-bit not DCLO and DCLZ; the NEC VR5500 has the full
complement of CLO, CLZ, DCLO and DCLZ.
> I see, cpu_has_clo_clz doesn't work well for platforms such as Malta.
> Malta can support several ISAs at a time, which is very valuable, but
> hard to be optimized :-)
While MIPS IV CPU cards for the Malta are available hardly anybody is using
on of those cards. Thus cpu_has_clo_clz defaults to cpu_has_mips_r and
ideally and platform should see cpu_has_mips_r to a constant to allow best
possible optimization. Malta doesn't ...
> > It might be simpler just to use __builtin_ffs() for this variant though.
> > Inline assembly is better avoided unless absolutely required. Not even
> > mentioning readability.
>
> Hm. It might be simpler, but it's not the purpose of irq_ffs(), IMHO.
Indeed.
Ralf
