Subject: Re: inlining spl*()
To: Chuck Silvers <chuq@chuq.com>
From: Chris G. Demetriou <cgd@sibyte.com>
List: port-mips
Date: 03/28/2001 09:41:01
> so assuming I'm not way off base here, the question I have is,
> do we want spl*() inlined all the time or only optionally?

I'd say provide them as extern inlines, i.e., also have a single
out-of-line copy, which will be used if for some reason inlining is
disabled.  (better than having them scattered all over as would be
done with static inlines.)  (Also, that may make life a little bit
better in terms of a sane LKM ABI -- not clear to me that LKMs should
really be trying to inline things like this anyway...)


some comments on your asms:

(1) you probably should be marking them volatile.

(2) it may make sense to do them multiline, rather than lots of asms
one right after the other.

> +static inline int
> +_splraise(int newipl)
> +{
> +	int streg;
> +
> +	__asm__("mfc0 %0, $12" : "=r" (streg));
> +	newipl &= MIPS_INT_MASK;
> +	__asm__("mtc0 %0, $12" : : "r"	(_splmask(streg & ~newipl)));
> +	__asm__("nop");
> +	return streg & (MIPS_INT_MASK | MIPS_SR_INT_IE);
> +}
> +
> +static inline int
> +_spllower(int newipl)
> +{
> +	int streg;
> +
> +	__asm__("mfc0 %0, $12" : "=r" (streg));
> +	__asm__("mtc0 %0, $12" : : "r" (_splmask((streg & ~MIPS_INT_MASK) |
> +						 (~newipl & MIPS_INT_MASK))));

I think you probably have to be careful about hazards in places like
this.  in particular, as i recall, on non-interlocked cpus there's a
one cycle hazard before the result register becomes valid ... and who
knows what the compiler's going to compile this into.

It's really probably best to translate the existing assembly directly
into single large asm statements, which use the necessary variables
and clobbers.

Also, there's no reason to encode the CP0 regs as hard-coded numbers
in the assembly.  you could, for instance, use:

    __asm__ ("mtc0 %0, $%1" : : "r"(val), "i"(CP0_REGNUM));

of course, the larger ASMs get the more confusing this becomes, but on
the other hand you're typically not whacking more than one or two cp0
regs in a single asm...


> +static inline void
> +_setsoftintr(int setbits)
> +{
> +	int streg, causereg;
> +
> +	__asm__("mfc0 %0, $12" : "=r" (streg));
> +	__asm__("mtc0 $0, $12");
> +	__asm__("nop; nop");
> +	__asm__("mfc0 %0, $13" : "=r" (causereg));
> +	__asm__("mtc0 %0, $13" : : "r" (causereg | setbits));

i think you have a hazard issue here too.

in general, there's lots of potential for hazards in code like this.
The two i pointed out are the two that i noticed, there are more.  (at
least one that i noticed while writing this paragraph, that i'm not
going to bother to point out.  You should just turn the assembly code
into C asms.)



cgd