code review 6256048: exp/locale/collate: avoid 16-bit math (issue 6256048)

[r...@golang.org]

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
exp/locale/collate: avoid 16-bit math

There's no need for the 16-bit arithmetic here,
and it tickles a long-standing compiler bug.
Fix the exp code not to use 16-bit math and
create an explicit test for the compiler bug.

Please review this at http://codereview.appspot.com/6256048/

Affected files:
M src/pkg/exp/locale/collate/colelem.go
A test/bugs/bug440.go


Index: src/pkg/exp/locale/collate/colelem.go
===================================================================
--- a/src/pkg/exp/locale/collate/colelem.go
+++ b/src/pkg/exp/locale/collate/colelem.go
@@ -102,7 +102,7 @@
)

func splitContractIndex(ce colElem) (index, n, offset int) {
- h := uint16(ce)
+ h := ce & 0xffff
return int(h >> maxNBits), int(h & (1<<maxNBits - 1)), int(ce>>16) &
(1<<maxContractOffsetBits - 1)
}

Index: test/bugs/bug440.go
===================================================================
new file mode 100644
--- /dev/null
+++ b/test/bugs/bug440.go
@@ -0,0 +1,21 @@
+// $G $D/$F.go && $L $F.$A && ./$A.out
+// # switch above to 'run' when bug gets fixed.
+// # right now it only breaks on 8g
+
+// Test for 8g register move bug. The optimizer gets confused
+// about 16- vs 32-bit moves during splitContractIndex.
+
+package main
+
+func main() {
+ const c = 0x12345678
+ index, n, offset := splitContractIndex(c)
+ if index != int((c&0xffff)>>5) || n != int(c & (1<<5-1)) || offset !=
(c>>16)&(1<<14-1) {
+ println("BUG", index, n, offset)
+ }
+}
+
+func splitContractIndex(ce uint32) (index, n, offset int) {
+ h := uint16(ce)
+ return int(h >> 5), int(h & (1<<5 - 1)), int(ce>>16) & (1<<14 - 1)
+}

[r...@golang.org]

LGTM

http://codereview.appspot.com/6256048/

[r...@golang.org]

LGTM

http://codereview.appspot.com/6256048/

[r...@golang.org]

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

exp/locale/collate: avoid 16-bit math

There's no need for the 16-bit arithmetic here,
and it tickles a long-standing compiler bug.
Fix the exp code not to use 16-bit math and
create an explicit test for the compiler bug.

R=golang-dev, r
CC=golang-dev
http://codereview.appspot.com/6256048


http://codereview.appspot.com/6256048/