Subject: Re: port-mips/26959: mips mc_cpuspeed() returns wrong value on gcc3
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
From: Simon Burge <simonb@wasabisystems.com>
List: netbsd-bugs
Date: 09/22/2004 17:17:11
Izumi Tsutsui wrote:

> In article <20040917021735.2B5CA23419@thoreau.thistledown.com.au>
> simonb@wasabisystems.com wrote:
> 
> > Can you try the following patch?
> 
> Unfortunately, it hangs before (at least) the first printf()
> in memsize_bitmap().

It was obviously put together a bit too hastily.  Sorry about that.

> > +LEAF_NOPROFILE(mips_mcclock_loop_with_clock)
> > +	move	t0,zero			# iters = 0;
> 
> I don't know MIPS ABI very well, but is t0 free here?
> Should we save s0 and use it?

t0-t9 are free for the function to use.

> > +	j	2f
> > +	 nop
> > +1:	nop				# asm ("nop;nop;nop;nop");
> > +	nop
> > +	nop
> > +	nop
> > +	addu	s0,1			# iters++;
> 
> This s0 (for iters) doesn't match the above "move t0,zero".

Yes, that s0 should be t0.

> > +2:	jal	mips_cp0_cause_read	# v0 = mips_cp0_cause_read();
> > +	 nop
> 
> If we call other functions, ra should be preserved?

Again, yes :-(

> > +	and	t0,a0			# v0 &= clockmask;
> > +	beqz	t0,1b			# if zero then repeat
> 
> These t0 should be v0 (returned value from mips_cp0_cause_read())?

Yes...

> > +	 move	 v0,t0			# return iters
> > +	j	ra
> 
> Some instruction is needed for delay slot here explicitely or not?

Once more yes...

> > +END(mips_mcclock_loop_with_clock)
> 
> 
> The following version works fine, but I'm not sure
> if the value "24" and register usage etc. are correct.

I was trying to get away from setting up a call frame.  We can "cheat"
because we know mips_cp0_cause_read() doesn't touch the temporary
registers but we may as well do it by the book.  I also thought about
just getting the cause register directly without the function call, but
that would mean recalculating the calibration constants.

The "24" is a part of the ABI - it's room for 4 argument regs, the stack
and ra.  There's a constant in <mips/asm.h> called CALLFRAME_SIZ we can
use.

> BTW, someone also pointed out that we should consider superscalar CPUs...

We'd need to make sure that we select the right mips cpu version to do
this.  "ssnop" isn't part of the standard mips1 and mips3 instruction
sets (however an "ssnop" is still valid when executed on earlier cpus -
it's just a shift to $zero).

Can you try the following?  The other change is move the "iters = 0"
into the delay slot.

LEAF_NOPROFILE(mips_mcclock_loop_with_clock)
	.set	noreorder
	subu	sp, CALLFRAME_SIZ
	sw	s0, 0(sp)
	sw	ra, CALLFRAME_RA(sp)
	j	2f
	 move	s0, zero		# iters = 0;
1:
	.set	push
	.set	mips32
	ssnop				# asm ("ssnop;ssnop;ssnop;ssnop");
	ssnop
	ssnop
	ssnop
	.set	pop
	addu	s0, 1			# iters++;
2:	jal	_C_LABEL(mips_cp0_cause_read)	# v0 = mips_cp0_cause_read();
	 nop
	and	v0, v0, a0		# v0 &= clockmask;
	beqz	v0, 1b			# if zero then repeat
	 move	 v0, s0			# return iters
	lw	ra, CALLFRAME_RA(sp)
	lw	s0, 0(sp)
	j	ra
	 addu	sp, CALLFRAME_SIZ
	.set	reorder
END(mips_mcclock_loop_with_clock)

Simon.
--
Simon Burge                            <simonb@wasabisystems.com>
NetBSD Support and Service:         http://www.wasabisystems.com/