Thank's for the findings. We verified the same issue on linux/s390x for go 1.21.4 and also the issue is not reproducing in the go1.20.11.
>From our initial findings of disassembled data, we are seeing an additional instruction getting emitted i.e. SLLG instruction on go1.21.4. This instruction is left shifting the operand logically by 24 bits. Due to this we feel that the output is incorrect. Below is the disassembled output for the "passAsIfaceAndThenDoOp" function. *For go 1.20.11:* 00000000000a8910 <main.passAsIfaceAndThenDoOp>: a8910: e3 30 d0 10 00 04 lg %r3,16(%r13) a8916: ec 3f 00 61 a0 65 clgrjnl %r3,%r15,a89d8 <main.passAsIfaceAndThenDoOp+0xc8> a891c: e3 e0 ff 98 ff 24 stg %r14,-104(%r15) a8922: e3 ff 0f 98 ff 71 lay %r15,-104(%r15,%r0) a8928: e3 e0 f0 00 00 24 stg %r14,0(%r15) a892e: e3 00 f0 70 00 04 lg %r0,112(%r15) a8934: c0 10 00 00 a1 66 larl %r1,bcc00 <type:*+0xcc00> a893a: ec 01 00 41 60 64 cgrjne %r0,%r1,a89bc <main.passAsIfaceAndThenDoOp+0xac> a8940: e3 10 f0 78 00 04 lg %r1,120(%r15) a8946: e3 00 10 00 00 04 lg %r0,0(%r1) a894c: e5 48 f0 58 00 00 mvghi 88(%r15),0 a8952: e5 48 f0 60 00 00 mvghi 96(%r15),0 a8958: e3 00 f0 08 00 24 stg %r0,8(%r15) a895e: c0 e5 ff fb 8f 69 brasl %r14,1a830 <runtime.convT64> a8964: e3 20 f0 10 00 04 lg %r2,16(%r15) a896a: c0 10 00 00 77 0b larl %r1,b7780 <type:*+0x7780> a8970: eb 12 f0 58 00 24 stmg %r1,%r2,88(%r15) a8976: c4 28 00 05 7b 0d lgrl %r2,157f90 <os.Stdout> a897c: c0 10 00 01 ec e6 larl %r1,e6348 <go:itab.*os.File,io.Writer> a8982: c0 30 00 01 16 4d larl %r3,cb61c <go:string.*+0x3fc4> a8988: eb 13 f0 08 00 24 stmg %r1,%r3,8(%r15) a898e: e5 48 f0 20 00 1a mvghi 32(%r15),26 a8994: 41 0f 00 58 la %r0,88(%r15,%r0) a8998: e3 00 f0 28 00 24 stg %r0,40(%r15) a899e: e5 48 f0 30 00 01 mvghi 48(%r15),1 a89a4: e5 48 f0 38 00 01 mvghi 56(%r15),1 a89aa: c0 e5 ff ff bb 03 brasl %r14,9ffb0 <fmt.Fprintf> a89b0: e3 e0 f0 00 00 04 lg %r14,0(%r15) a89b6: a7 fb 00 68 aghi %r15,104 a89ba: 07 fe br %r14 a89bc: b9 04 00 21 lgr %r2,%r1 a89c0: c0 30 00 00 83 30 larl %r3,b9020 <type:*+0x9020> a89c6: b9 04 00 10 lgr %r1,%r0 a89ca: eb 13 f0 08 00 24 stmg %r1,%r3,8(%r15) a89d0: c0 e5 ff fb 8d b0 brasl %r14,1a530 <runtime.panicdottypeE> a89d6: 07 00 nopr a89d8: b9 04 00 5e lgr %r5,%r14 a89dc: c0 e5 ff fe 9f 0a brasl %r14,7c7f0 <runtime.morestack_noctxt> a89e2: a7 f4 ff 97 j a8910 <main.passAsIfaceAndThenDoOp> *For go 1.21.4:* 00000000000a8250 <main.passAsIfaceAndThenDoOp>: a8250: e3 30 d0 10 00 04 lg %r3,16(%r13) a8256: ec 3f 00 64 a0 65 clgrjnl %r3,%r15,a831e <main.passAsIfaceAndThenDoOp+0xce> a825c: e3 e0 ff 98 ff 24 stg %r14,-104(%r15) a8262: e3 ff 0f 98 ff 71 lay %r15,-104(%r15,%r0) a8268: e3 e0 f0 00 00 24 stg %r14,0(%r15) a826e: e3 00 f0 70 00 04 lg %r0,112(%r15) a8274: c0 10 00 00 a7 16 larl %r1,bd0a0 <type:*+0xd0a0> a827a: ec 01 00 44 60 64 cgrjne %r0,%r1,a8302 <main.passAsIfaceAndThenDoOp+0xb2> a8280: e3 10 f0 78 00 04 lg %r1,120(%r15) a8286: e3 00 10 00 00 04 lg %r0,0(%r1) *a828c: eb 00 00 18 00 0d sllg %r0,%r0,2*4 ----------------------------->>>>>>>>>>>>>>>>>>>>>>>>>>> This is the extra instruction which is getting emitted in the case of go 1.21 a8292: e5 48 f0 58 00 00 mvghi 88(%r15),0 a8298: e5 48 f0 60 00 00 mvghi 96(%r15),0 a829e: e3 00 f0 08 00 24 stg %r0,8(%r15) a82a4: c0 e5 ff fb 94 be brasl %r14,1ac20 <runtime.convT64> a82aa: e3 20 f0 10 00 04 lg %r2,16(%r15) a82b0: c0 10 00 00 77 b8 larl %r1,b7220 <type:*+0x7220> a82b6: eb 12 f0 58 00 24 stmg %r1,%r2,88(%r15) a82bc: c4 28 00 05 89 26 lgrl %r2,159508 <os.Stdout> a82c2: c0 10 00 01 ff bb larl %r1,e8238 <go:itab.*os.File,io.Writer> a82c8: c0 30 00 01 1d 5a larl %r3,cbd7c <go:string.*+0x3884> a82ce: eb 13 f0 08 00 24 stmg %r1,%r3,8(%r15) a82d4: e5 48 f0 20 00 1a mvghi 32(%r15),26 a82da: 41 0f 00 58 la %r0,88(%r15,%r0) a82de: e3 00 f0 28 00 24 stg %r0,40(%r15) a82e4: e5 48 f0 30 00 01 mvghi 48(%r15),1 a82ea: e5 48 f0 38 00 01 mvghi 56(%r15),1 a82f0: c0 e5 ff ff ba 78 brasl %r14,9f7e0 <fmt.Fprintf> a82f6: e3 e0 f0 00 00 04 lg %r14,0(%r15) a82fc: a7 fb 00 68 aghi %r15,104 a8300: 07 fe br %r14 a8302: b9 04 00 21 lgr %r2,%r1 a8306: c0 30 00 00 89 dd larl %r3,b96c0 <type:*+0x96c0> a830c: b9 04 00 10 lgr %r1,%r0 a8310: eb 13 f0 08 00 24 stmg %r1,%r3,8(%r15) a8316: c0 e5 ff fb 93 2d brasl %r14,1a970 <runtime.panicdottypeE> a831c: 07 00 nopr a831e: b9 04 00 5e lgr %r5,%r14 a8322: c0 e5 ff fe be 17 brasl %r14,7ff50 <runtime.morestack_noctxt> a8328: a7 f4 ff 94 j a8250 <main.passAsIfaceAndThenDoOp> As you can see above that the SLLG instruction is logically left shifting the second operand (r0) by 24bits and placing it in the first operand (r0). Due to this, the 64 bit contents of "r0" register is becoming "00ABCDEF12000000".... Ex: r0: ==> 0x00000000ABCDEFf12 After SLLG instruction sllg %r0, %r0, 24 The r0 becomes ==> 0x00ABCDEF12000000 After this we returning the r0 content. We are further taking a look at the same. We will share our further findings when we have it. ~ On Wednesday, November 29, 2023 at 8:10:39 PM UTC+5:30 Timothy Olsen wrote: > I can confirm that it is six zeroes. That is indeed weird and highlights > that it's not just a simple switching of the fields. > > I will file a ticket now. Thank you! > > -Tim > > On Tue, Nov 28, 2023 at 8:30 PM 'Keith Randall' via golang-nuts < > [email protected]> wrote: > >> It seems strange that the bad result is ABCDEF12000000 and not >> ABCDEF1200000000. i.e., 6 zeros and not 8. Can you confirm? >> >> Definitely sounds like a bug to me. You should open an issue. >> On Tuesday, November 28, 2023 at 1:44:59 PM UTC-8 Timothy Olsen wrote: >> >>> A coworker suggested I try with optimizations off: >>> >>> [[email protected] ~]$ go version >>> go version go1.21.4 linux/s390x >>> [[email protected] ~]$ go build -o /tmp/scratch_1 >>> scratch_1.go >>> [[email protected] ~]$ /tmp/scratch_1 >>> ABCDEF12 >>> ABCDEF12000000 >>> [[email protected] ~]$ go build -gcflags='-N' -o >>> /tmp/scratch_1 scratch_1.go >>> [[email protected] ~]$ /tmp/scratch_1 >>> ABCDEF12 >>> ABCDEF12 >>> [[email protected] ~]$ export >>> GOROOT=/opt/golang/go1.20.11 >>> [[email protected] ~]$ export >>> PATH=${GOROOT}/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/opt/ibm/java-s390x-80/bin:/home/tolsen/.local/bin:/home/tolsen/bin >>> [[email protected] ~]$ go build -o /tmp/scratch_1 >>> scratch_1.go >>> [[email protected] ~]$ /tmp/scratch_1 >>> ABCDEF12 >>> ABCDEF12 >>> [[email protected] ~]$ >>> >>> That further confirms a bug with optimizations on s390x (and possibly >>> other big endian machines?) . >>> >>> -Tim >>> >>> On Tue, Nov 28, 2023 at 4:36 PM '[email protected]' via golang-nuts < >>> [email protected]> wrote: >>> >>>> Hello, >>>> >>>> I believe I've found a code-optimization bug in Go 1.21.4 on Linux >>>> s390x. This is what I was able to narrow the code sample down to: >>>> >>>> ////////// >>>> package main >>>> >>>> import "fmt" >>>> >>>> type myStruct struct { >>>> A uint32 >>>> B uint32 >>>> } >>>> >>>> func doOpOnStructElems(a, b uint32) uint64 { >>>> return (uint64(a) << 32) | uint64(b) >>>> } >>>> >>>> func main() { >>>> myVal := myStruct{0, 0xABCDEF12} >>>> >>>> passAsMyStructAndThenDoOp(myVal) >>>> passAsIfaceAndThenDoOp(myVal) >>>> } >>>> >>>> func passAsMyStructAndThenDoOp(myVal myStruct) { >>>> fmt.Printf("%X\n", doOpOnStructElems(myVal.A, myVal.B)) >>>> } >>>> >>>> func passAsIfaceAndThenDoOp(myIface interface{}) { >>>> fmt.Printf("%X\n", doOpOnStructElems(myIface.(myStruct).A, >>>> myIface.(myStruct).B)) >>>> } >>>> //////// >>>> >>>> When I run it I get: >>>> >>>> ///// >>>> >>>> $ go run scratch_1.go >>>> >>>> ABCDEF12 >>>> >>>> ABCDEF12000000 >>>> ////// >>>> >>>> If I run it on Linux zSeries w/ Go 1.20.11 or on Linux AMD64, Linux >>>> ARM64, or macOS w/ Go 1.21.4, I get what I believe is the correct answer: >>>> >>>> //// >>>> >>>> $ go run scratch_1.go >>>> >>>> ABCDEF12 >>>> >>>> ABCDEF12 >>>> >>>> //// >>>> >>>> In other words, with Go 1.21.4 and s390x it appears that A & B are >>>> switched in the 2nd call which passes the struct as an interface{} . >>>> >>>> s390x is the only big endian platform I am able to test on. So I >>>> suspect there may be an endianness issue here. I suspect something has >>>> gone wrong with some sort of code optimization because if I insert >>>> Println() at the beginning of doOpOnStructElems(), the problem goes away. >>>> So it's possible there's some bug with code inlining when what is being >>>> passed in was originally passed in as an interface in the caller? >>>> >>>> If someone could confirm that this is indeed a bug I will be happy to >>>> file an issue. >>>> >>>> Thank you, >>>> >>>> Tim >>>> >>> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "golang-nuts" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/golang-nuts/TYGuzUTTsN8/unsubscribe. >> To unsubscribe from this group and all its topics, send an email to >> [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/golang-nuts/b2a37dfc-7b6a-412a-8cbb-40a2210d6921n%40googlegroups.com >> >> <https://groups.google.com/d/msgid/golang-nuts/b2a37dfc-7b6a-412a-8cbb-40a2210d6921n%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> > -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/40f64bb8-481d-4919-ac68-dbd9b231b51fn%40googlegroups.com.
