code review 174052: first stub for softfloats, intercepts float instructio...

first stub for softfloats, intercepts float instructions and skips
        them in the stream.

[kaib, 2009-12-11 00:31:06]

Hello rsc,

I'd like you to review the following change.

[rsc, 2009-12-11 02:26:35]

http://codereview.appspot.com/174052/diff/1008/10
File src/cmd/5l/l.h (right):

http://codereview.appspot.com/174052/diff/1008/10#newcode454
src/cmd/5l/l.h:454: void softfloat(void);
s/ /\t/
sorry

http://codereview.appspot.com/174052/diff/1008/15
File src/pkg/runtime/arm/asm.s (right):

http://codereview.appspot.com/174052/diff/1008/15#newcode78
src/pkg/runtime/arm/asm.s:78: TEXT _dep_dummy(SB),7,$0
you never refer to _dep_dummy, right?
deadcode would throw this away and not
use the references to hold anything up.
i think i know what's wrong - it's not that the loader
is throwing away the object in deadcode but that
it doesn't realize it needs to pull it in at all.

the bottom of ld/lib.c:/^libinit does 
	lookup(INITENTRY, 0)->type = SXREF;
and i think that 5l needs to do a similar
thing for each of these names, so that when
pulling in functions from the archive, it realizes
that it needs them.

http://codereview.appspot.com/174052/diff/1008/15#newcode84
src/pkg/runtime/arm/asm.s:84: BL	runtime·softfloat(SB)
might as well call this _float like the others.

http://codereview.appspot.com/174052/diff/1008/15#newcode244
src/pkg/runtime/arm/asm.s:244: TEXT	runtime·softfloat(SB), 7, $4
i suggest moving this into vlop.s so that all the
math simulation is in the same place.

on a related note, i think you need to BL save<>(SB) and B out like _divu and
_modu do, to save the registers across the call.
(the code generator did not expect the fp instruction to
trash all the registers.)

http://codereview.appspot.com/174052/diff/1008/16
File src/pkg/runtime/arm/softfloat.c (right):

http://codereview.appspot.com/174052/diff/1008/16#newcode14
src/pkg/runtime/arm/softfloat.c:14: }
odd spacing.
looks like maybe spaces instead of a tab.

[kaib, 2009-12-14 23:59:19]

there is some more work in softfloat.c to support skipping cases where we
generated multiple arm instructions for a single float instruction (MOVF).

this now works on arm:
fmt.Printf("Hello World\n");

http://codereview.appspot.com/174052/diff/1008/10
File src/cmd/5l/l.h (right):

http://codereview.appspot.com/174052/diff/1008/10#newcode454
src/cmd/5l/l.h:454: void softfloat(void);
On 2009/12/11 02:26:35, rsc wrote:
> s/ /\t/
> sorry
> 

Done.

http://codereview.appspot.com/174052/diff/1008/15
File src/pkg/runtime/arm/asm.s (right):

http://codereview.appspot.com/174052/diff/1008/15#newcode78
src/pkg/runtime/arm/asm.s:78: TEXT _dep_dummy(SB),7,$0
On 2009/12/11 02:26:35, rsc wrote:
> you never refer to _dep_dummy, right?

it actually is referenced on line 61

> deadcode would throw this away and not
> use the references to hold anything up.
> i think i know what's wrong - it's not that the loader
> is throwing away the object in deadcode but that
> it doesn't realize it needs to pull it in at all.
> 
> the bottom of ld/lib.c:/^libinit does 
> 	lookup(INITENTRY, 0)->type = SXREF;
> and i think that 5l needs to do a similar
> thing for each of these names, so that when
> pulling in functions from the archive, it realizes
> that it needs them.

i'll try this, this sounds a lot like what is happening. ie. when it's
referenced here it gets SXREF set but not when just  marked.

http://codereview.appspot.com/174052/diff/1008/15#newcode84
src/pkg/runtime/arm/asm.s:84: BL	runtime·softfloat(SB)
On 2009/12/11 02:26:35, rsc wrote:
> might as well call this _float like the others.

called it _sfloat and _sfloat2

http://codereview.appspot.com/174052/diff/1008/15#newcode244
src/pkg/runtime/arm/asm.s:244: TEXT	runtime·softfloat(SB), 7, $4
On 2009/12/11 02:26:35, rsc wrote:
> i suggest moving this into vlop.s so that all the
> math simulation is in the same place.
> 
> on a related note, i think you need to BL save<>(SB) and B out like _divu and
> _modu do, to save the registers across the call.
> (the code generator did not expect the fp instruction to
> trash all the registers.)

Done.

http://codereview.appspot.com/174052/diff/1008/16
File src/pkg/runtime/arm/softfloat.c (right):

http://codereview.appspot.com/174052/diff/1008/16#newcode14
src/pkg/runtime/arm/softfloat.c:14: }
On 2009/12/11 02:26:35, rsc wrote:
> odd spacing.
> looks like maybe spaces instead of a tab.
> 

Done.

[rsc, 2009-12-15 00:05:13]

cool.

http://codereview.appspot.com/174052/diff/3010/3018
File src/pkg/runtime/arm/softfloat.c (right):

http://codereview.appspot.com/174052/diff/3010/3018#newcode11
src/pkg/runtime/arm/softfloat.c:11: uint32 i = *pc;
typical local C convention is to keep
declaration separate from initialization,
start if and while bodies on their own lines,
and drop the space after the words if, for, switch.

http://codereview.appspot.com/174052/diff/3010/3018#newcode37
src/pkg/runtime/arm/softfloat.c:37: uint32*
before this line, insert

#pragma textflag 7

so that there is no possibility of _sfloat2
causing a stack split.  (if the stack split,
only r0 would be copied, and then &r0 
would not be what you want it to mean.)

[kaib, 2009-12-15 00:26:06]

ptal

http://codereview.appspot.com/174052/diff/3010/3018
File src/pkg/runtime/arm/softfloat.c (right):

http://codereview.appspot.com/174052/diff/3010/3018#newcode11
src/pkg/runtime/arm/softfloat.c:11: uint32 i = *pc;
On 2009/12/15 00:05:13, rsc wrote:
> typical local C convention is to keep
> declaration separate from initialization,
> start if and while bodies on their own lines,
> and drop the space after the words if, for, switch.
> 

done. my bad for the bother.

http://codereview.appspot.com/174052/diff/3010/3018#newcode37
src/pkg/runtime/arm/softfloat.c:37: uint32*
On 2009/12/15 00:05:13, rsc wrote:
> before this line, insert
> 
> #pragma textflag 7
> 
> so that there is no possibility of _sfloat2
> causing a stack split.  (if the stack split,
> only r0 would be copied, and then &r0 
> would not be what you want it to mean.)
> 

done. good catch.

[kaib, 2009-12-16 03:59:30]

ping

On 2009/12/15 00:26:06, kaib wrote:
> ptal
> 
> http://codereview.appspot.com/174052/diff/3010/3018
> File src/pkg/runtime/arm/softfloat.c (right):
> 
> http://codereview.appspot.com/174052/diff/3010/3018#newcode11
> src/pkg/runtime/arm/softfloat.c:11: uint32 i = *pc;
> On 2009/12/15 00:05:13, rsc wrote:
> > typical local C convention is to keep
> > declaration separate from initialization,
> > start if and while bodies on their own lines,
> > and drop the space after the words if, for, switch.
> > 
> 
> done. my bad for the bother.
> 
> http://codereview.appspot.com/174052/diff/3010/3018#newcode37
> src/pkg/runtime/arm/softfloat.c:37: uint32*
> On 2009/12/15 00:05:13, rsc wrote:
> > before this line, insert
> > 
> > #pragma textflag 7
> > 
> > so that there is no possibility of _sfloat2
> > causing a stack split.  (if the stack split,
> > only r0 would be copied, and then &r0 
> > would not be what you want it to mean.)
> > 
> 
> done. good catch.

[rsc, 2009-12-16 04:32:14]

Looks like 5l/softfloat.c is missing?

http://codereview.appspot.com/174052/diff/2021/3025
File src/cmd/5l/obj.c (right):

http://codereview.appspot.com/174052/diff/2021/3025#newcode267
src/cmd/5l/obj.c:267: // TODO(kaib): this doesn't work, the prog can't be found
in runtime
did you fix this with the SXREF?

[kaib, 2009-12-16 05:26:01]

On 2009/12/16 04:32:14, rsc wrote:
> Looks like 5l/softfloat.c is missing?

done. my hg-fu needs upgrading ..

http://codereview.appspot.com/174052/diff/2021/3025#newcode267
> src/cmd/5l/obj.c:267: // TODO(kaib): this doesn't work, the prog can't be
found
> in runtime
> did you fix this with the SXREF?

no, added todo on my 5g list

[rsc, 2009-12-18 00:04:04]

LGTM

[kaib, 2009-12-18 00:08:43]

*** Submitted as http://code.google.com/p/go/source/detail?r=427b3407ebd8 ***

	first stub for softfloats, intercepts float instructions and skips
        them in the stream.

R=rsc
http://codereview.appspot.com/174052