From ef46a9ddacb25bc9155c1dcb74e61995b2e3dc39 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sun, 15 Nov 2009 17:24:14 -0800 Subject: [PATCH] gc: fix up floating point NaN comparisons Fixes #167. R=ken2 https://golang.org/cl/155062 --- src/cmd/5g/cgen.c | 13 ++++++- src/cmd/6g/cgen.c | 55 +++++++++++++++++++++-------- src/cmd/6g/gsubr.c | 4 --- src/cmd/8g/cgen.c | 47 +++++++++++++++++++------ src/cmd/8g/gsubr.c | 4 --- test/float_lit.go | 8 ++--- test/floatcmp.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 181 insertions(+), 38 deletions(-) create mode 100644 test/floatcmp.go diff --git a/src/cmd/5g/cgen.c b/src/cmd/5g/cgen.c index 9fc59391e9..019704c98a 100644 --- a/src/cmd/5g/cgen.c +++ b/src/cmd/5g/cgen.c @@ -895,8 +895,19 @@ bgen(Node *n, int true, Prog *to) case OLE: case OGE: a = n->op; - if(!true) + if(!true) { + if(isfloat[nl->type->etype]) { + // brcom is not valid on floats when NaN is involved. + p1 = gbranch(AJMP, T); + p2 = gbranch(AJMP, T); + patch(p1, pc); + bgen(n, 1, p2); + patch(gbranch(AJMP, T), to); + patch(p2, pc); + goto ret; + } a = brcom(a); + } // make simplest on right if(nl->op == OLITERAL || nl->ullman < nr->ullman) { diff --git a/src/cmd/6g/cgen.c b/src/cmd/6g/cgen.c index 2ee7934908..041f6c13c0 100644 --- a/src/cmd/6g/cgen.c +++ b/src/cmd/6g/cgen.c @@ -668,7 +668,7 @@ void bgen(Node *n, int true, Prog *to) { int et, a; - Node *nl, *nr, *r; + Node *nl, *nr, *l, *r; Node n1, n2, tmp; Prog *p1, *p2; @@ -782,8 +782,19 @@ bgen(Node *n, int true, Prog *to) case OLE: case OGE: a = n->op; - if(!true) + if(!true) { + if(isfloat[nr->type->etype]) { + // brcom is not valid on floats when NaN is involved. + p1 = gbranch(AJMP, T); + p2 = gbranch(AJMP, T); + patch(p1, pc); + bgen(n, 1, p2); + patch(gbranch(AJMP, T), to); + patch(p2, pc); + goto ret; + } a = brcom(a); + } // make simplest on right if(nl->op == OLITERAL || nl->ullman < nr->ullman) { @@ -792,7 +803,7 @@ bgen(Node *n, int true, Prog *to) nl = nr; nr = r; } - + if(isslice(nl->type)) { // only valid to cmp darray to literal nil if((a != OEQ && a != ONE) || nr->op != OLITERAL) { @@ -831,8 +842,6 @@ bgen(Node *n, int true, Prog *to) break; } - a = optoas(a, nr->type); - if(nr->ullman >= UINF) { regalloc(&n1, nr->type, N); cgen(nr, &n1); @@ -847,12 +856,7 @@ bgen(Node *n, int true, Prog *to) regalloc(&n2, nr->type, &n2); cgen(&tmp, &n2); - gins(optoas(OCMP, nr->type), &n1, &n2); - patch(gbranch(a, nr->type), to); - - regfree(&n1); - regfree(&n2); - break; + goto cmp; } regalloc(&n1, nl->type, N); @@ -860,17 +864,40 @@ bgen(Node *n, int true, Prog *to) if(smallintconst(nr)) { gins(optoas(OCMP, nr->type), &n1, nr); - patch(gbranch(a, nr->type), to); + patch(gbranch(optoas(a, nr->type), nr->type), to); regfree(&n1); break; } regalloc(&n2, nr->type, N); cgen(nr, &n2); + cmp: + // only < and <= work right with NaN; reverse if needed + l = &n1; + r = &n2; + if(isfloat[nl->type->etype] && (a == OGT || a == OGE)) { + l = &n2; + r = &n1; + a = brrev(a); + } - gins(optoas(OCMP, nr->type), &n1, &n2); - patch(gbranch(a, nr->type), to); + gins(optoas(OCMP, nr->type), l, r); + if(isfloat[nr->type->etype] && (n->op == OEQ || n->op == ONE)) { + if(n->op == OEQ) { + // neither NE nor P + p1 = gbranch(AJNE, T); + p2 = gbranch(AJPS, T); + patch(gbranch(AJMP, T), to); + patch(p1, pc); + patch(p2, pc); + } else { + // either NE or P + patch(gbranch(AJNE, T), to); + patch(gbranch(AJPS, T), to); + } + } else + patch(gbranch(optoas(a, nr->type), nr->type), to); regfree(&n1); regfree(&n2); break; diff --git a/src/cmd/6g/gsubr.c b/src/cmd/6g/gsubr.c index 20b79c0be9..4f3c85a6b3 100644 --- a/src/cmd/6g/gsubr.c +++ b/src/cmd/6g/gsubr.c @@ -1146,8 +1146,6 @@ optoas(int op, Type *t) case CASE(OLT, TUINT16): case CASE(OLT, TUINT32): case CASE(OLT, TUINT64): - case CASE(OGT, TFLOAT32): - case CASE(OGT, TFLOAT64): a = AJCS; break; @@ -1162,8 +1160,6 @@ optoas(int op, Type *t) case CASE(OLE, TUINT16): case CASE(OLE, TUINT32): case CASE(OLE, TUINT64): - case CASE(OGE, TFLOAT32): - case CASE(OGE, TFLOAT64): a = AJLS; break; diff --git a/src/cmd/8g/cgen.c b/src/cmd/8g/cgen.c index bf0b263b61..ee4df870a1 100644 --- a/src/cmd/8g/cgen.c +++ b/src/cmd/8g/cgen.c @@ -838,8 +838,19 @@ bgen(Node *n, int true, Prog *to) case OLE: case OGE: a = n->op; - if(!true) + if(!true) { + if(isfloat[nl->type->etype]) { + // brcom is not valid on floats when NaN is involved. + p1 = gbranch(AJMP, T); + p2 = gbranch(AJMP, T); + patch(p1, pc); + bgen(n, 1, p2); + patch(gbranch(AJMP, T), to); + patch(p2, pc); + break; + } a = brcom(a); + } // make simplest on right if(nl->op == OLITERAL || nl->ullman < nr->ullman) { @@ -888,6 +899,14 @@ bgen(Node *n, int true, Prog *to) } if(isfloat[nr->type->etype]) { + a = brrev(a); // because the args are stacked + if(a == OGE || a == OGT) { + // only < and <= work right with NaN; reverse if needed + r = nr; + nr = nl; + nl = r; + a = brrev(a); + } nodreg(&tmp, nr->type, D_F0); nodreg(&n2, nr->type, D_F0 + 1); nodreg(&ax, types[TUINT16], D_AX); @@ -915,7 +934,19 @@ bgen(Node *n, int true, Prog *to) } gins(AFSTSW, N, &ax); gins(ASAHF, N, N); - patch(gbranch(optoas(brrev(a), nr->type), T), to); + if(a == OEQ) { + // neither NE nor P + p1 = gbranch(AJNE, T); + p2 = gbranch(AJPS, T); + patch(gbranch(AJMP, T), to); + patch(p1, pc); + patch(p2, pc); + } else if(a == ONE) { + // either NE or P + patch(gbranch(AJNE, T), to); + patch(gbranch(AJPS, T), to); + } else + patch(gbranch(optoas(a, nr->type), T), to); break; } @@ -941,21 +972,14 @@ bgen(Node *n, int true, Prog *to) a = optoas(a, nr->type); if(nr->ullman >= UINF) { + tempalloc(&n1, nl->type); tempalloc(&tmp, nr->type); cgen(nr, &tmp); - - tempalloc(&n1, nl->type); cgen(nl, &n1); - regalloc(&n2, nr->type, N); cgen(&tmp, &n2); - - gins(optoas(OCMP, nr->type), &n1, &n2); - patch(gbranch(a, nr->type), to); - tempfree(&n1); tempfree(&tmp); - regfree(&n2); - break; + goto cmp; } tempalloc(&n1, nl->type); @@ -974,6 +998,7 @@ bgen(Node *n, int true, Prog *to) gmove(&tmp, &n2); tempfree(&tmp); +cmp: gins(optoas(OCMP, nr->type), &n1, &n2); patch(gbranch(a, nr->type), to); regfree(&n2); diff --git a/src/cmd/8g/gsubr.c b/src/cmd/8g/gsubr.c index dea802f4e1..71a7494fc2 100755 --- a/src/cmd/8g/gsubr.c +++ b/src/cmd/8g/gsubr.c @@ -260,8 +260,6 @@ optoas(int op, Type *t) case CASE(OLT, TUINT16): case CASE(OLT, TUINT32): case CASE(OLT, TUINT64): - case CASE(OGT, TFLOAT32): - case CASE(OGT, TFLOAT64): a = AJCS; break; @@ -276,8 +274,6 @@ optoas(int op, Type *t) case CASE(OLE, TUINT16): case CASE(OLE, TUINT32): case CASE(OLE, TUINT64): - case CASE(OGE, TFLOAT32): - case CASE(OGE, TFLOAT64): a = AJLS; break; diff --git a/test/float_lit.go b/test/float_lit.go index a78a6e9245..be4460e43e 100644 --- a/test/float_lit.go +++ b/test/float_lit.go @@ -20,8 +20,8 @@ close(da float64, ia, ib int64, pow int) bool db := float64(ia) / float64(ib); db *= pow10(pow); - if da == 0 { - if db == 0 { + if da == 0 || db == 0 { + if da == 0 && db == 0 { return true; } return false; @@ -59,8 +59,8 @@ main() if !close(-210e3, -210, 1, 3) { print("-210e3 is ", -210e3, "\n"); } if !close(0E-1, 0, 1, 0) { print("0E-1 is ", 0E-1, "\n"); } - if !close(+0e23, 0, 1, 23) { print("+0e23 is ", +0e23, "\n"); } - if !close(-0e345, 0, 1, 345) { print("-0e345 is ", -0e345, "\n"); } + if !close(+0e23, 0, 1, 1) { print("+0e23 is ", +0e23, "\n"); } + if !close(-0e345, 0, 1, 1) { print("-0e345 is ", -0e345, "\n"); } if !close(0E1, 0, 1, 1) { print("0E1 is ", 0E1, "\n"); } if !close(+10e23, 10, 1, 23) { print("+10e23 is ", +10e23, "\n"); } diff --git a/test/floatcmp.go b/test/floatcmp.go new file mode 100644 index 0000000000..26fc6ad14c --- /dev/null +++ b/test/floatcmp.go @@ -0,0 +1,88 @@ +// $G $F.go && $L $F.$A && ./$A.out + +// Copyright 2009 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import "math" + +type floatTest struct { + name string; + expr bool; + want bool; +} + +var nan float64 = math.NaN(); +var f float64 = 1; + +var tests = []floatTest{ + floatTest{"nan == nan", nan == nan, false}, + floatTest{"nan != nan", nan != nan, true}, + floatTest{"nan < nan", nan < nan, false}, + floatTest{"nan > nan", nan > nan, false}, + floatTest{"nan <= nan", nan <= nan, false}, + floatTest{"nan >= nan", nan >= nan, false}, + floatTest{"f == nan", f == nan, false}, + floatTest{"f != nan", f != nan, true}, + floatTest{"f < nan", f < nan, false}, + floatTest{"f > nan", f > nan, false}, + floatTest{"f <= nan", f <= nan, false}, + floatTest{"f >= nan", f >= nan, false}, + floatTest{"nan == f", nan == f, false}, + floatTest{"nan != f", nan != f, true}, + floatTest{"nan < f", nan < f, false}, + floatTest{"nan > f", nan > f, false}, + floatTest{"nan <= f", nan <= f, false}, + floatTest{"nan >= f", nan >= f, false}, + floatTest{"!(nan == nan)", !(nan == nan), true}, + floatTest{"!(nan != nan)", !(nan != nan), false}, + floatTest{"!(nan < nan)", !(nan < nan), true}, + floatTest{"!(nan > nan)", !(nan > nan), true}, + floatTest{"!(nan <= nan)", !(nan <= nan), true}, + floatTest{"!(nan >= nan)", !(nan >= nan), true}, + floatTest{"!(f == nan)", !(f == nan), true}, + floatTest{"!(f != nan)", !(f != nan), false}, + floatTest{"!(f < nan)", !(f < nan), true}, + floatTest{"!(f > nan)", !(f > nan), true}, + floatTest{"!(f <= nan)", !(f <= nan), true}, + floatTest{"!(f >= nan)", !(f >= nan), true}, + floatTest{"!(nan == f)", !(nan == f), true}, + floatTest{"!(nan != f)", !(nan != f), false}, + floatTest{"!(nan < f)", !(nan < f), true}, + floatTest{"!(nan > f)", !(nan > f), true}, + floatTest{"!(nan <= f)", !(nan <= f), true}, + floatTest{"!(nan >= f)", !(nan >= f), true}, + floatTest{"!!(nan == nan)", !!(nan == nan), false}, + floatTest{"!!(nan != nan)", !!(nan != nan), true}, + floatTest{"!!(nan < nan)", !!(nan < nan), false}, + floatTest{"!!(nan > nan)", !!(nan > nan), false}, + floatTest{"!!(nan <= nan)", !!(nan <= nan), false}, + floatTest{"!!(nan >= nan)", !!(nan >= nan), false}, + floatTest{"!!(f == nan)", !!(f == nan), false}, + floatTest{"!!(f != nan)", !!(f != nan), true}, + floatTest{"!!(f < nan)", !!(f < nan), false}, + floatTest{"!!(f > nan)", !!(f > nan), false}, + floatTest{"!!(f <= nan)", !!(f <= nan), false}, + floatTest{"!!(f >= nan)", !!(f >= nan), false}, + floatTest{"!!(nan == f)", !!(nan == f), false}, + floatTest{"!!(nan != f)", !!(nan != f), true}, + floatTest{"!!(nan < f)", !!(nan < f), false}, + floatTest{"!!(nan > f)", !!(nan > f), false}, + floatTest{"!!(nan <= f)", !!(nan <= f), false}, + floatTest{"!!(nan >= f)", !!(nan >= f), false}, +} + +func main() { + bad := false; + for _, t := range tests { + if t.expr != t.want { + if !bad { + bad = true; + println("BUG: floatcmp"); + } + println(t.name, "=", t.expr, "want", t.want); + } + } +}