code review 2540042: runtime: fix arm softfloat again for R12

runtime: fix arm softfloat again for R12

Missed a case, so now generalized.

[rsc, 2010-10-18 17:24:17]

Hello ken2 (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[rsc, 2010-10-18 17:24:26]

*** Submitted as 9f3acaf1a4e8 ***

runtime: fix arm softfloat again for R12

Missed a case, so now generalized.

R=ken2
CC=golang-dev
http://codereview.appspot.com/2540042

[rsc, 2010-10-18 17:29:44]

> +++ b/src/pkg/runtime/arm/softfloat.c
> @@ -449,7 +449,7 @@
> -       tlen = i>>(22 - 1)&1 | i>>15&1;
> +       tlen = i>>(22 - 1)&1 | i>>15&1; // NOTE(rsc): should this say
i>>(22-1)&2 (not &1)?

well kai, should it?  :-)
as written tlen is an | of two values <= 1,
but then there is a check for tlen > 1 below.
also as written the first half of the | is the same as wb.

russ

[ken3, 2010-10-18 22:07:04]

lgtm

[kaib, 2010-10-19 09:54:20]

On Mon, Oct 18, 2010 at 8:29 PM, Russ Cox <rsc@golang.org> wrote:
>> +++ b/src/pkg/runtime/arm/softfloat.c
>> @@ -449,7 +449,7 @@
>> -       tlen = i>>(22 - 1)&1 | i>>15&1;
>> +       tlen = i>>(22 - 1)&1 | i>>15&1; // NOTE(rsc): should this say
i>>(22-1)&2 (not &1)?
>
> well kai, should it?  :-)
> as written tlen is an | of two values <= 1,
> but then there is a check for tlen > 1 below.
> also as written the first half of the | is the same as wb.

it should be i>>(22-1)&2. that said, if the t1 bit (22) is set the
format is either extended, packed decimal or extended packed decimal.
none of those are supported so we do the tlen > 1 check and go to
undef. this is really a sanity check, the linker should never generate
code with t1 set..

 kai