linux-mips
[Top] [All Lists]

Re: [PATCH] MIPS: PowerTV: Use fls() carefully where static optimization

To: "Maciej W. Rozycki" <macro@linux-mips.org>
Subject: Re: [PATCH] MIPS: PowerTV: Use fls() carefully where static optimization is required
From: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
Date: Mon, 05 Jul 2010 09:33:36 +0900
Cc: Shinya Kuribayashi <skuribay@pobox.com>, David VomLehn <dvomlehn@cisco.com>, linux-mips@linux-mips.org
In-reply-to: <alpine.LFD.2.00.1007031748350.11778@eddie.linux-mips.org>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <4C2755A3.3080600@pobox.com> <20100630220124.GA576@dvomlehn-lnx2.corp.sa.net> <4C2DF427.7080508@pobox.com> <20100702213219.GA390@dvomlehn-lnx2.corp.sa.net> <4C2F49D0.60200@pobox.com> <alpine.LFD.2.00.1007031748350.11778@eddie.linux-mips.org>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4) Gecko/20100608 Lightning/1.0b2 Thunderbird/3.1
Hi,

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.

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 :-)

>  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.

-- 
Shinya Kuribayashi
Renesas Electronics

<Prev in Thread] Current Thread [Next in Thread>