Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/compile: inconsistent nil check elimination on ppc64 #9058

Closed
gopherbot opened this issue Nov 3, 2014 · 4 comments
Closed

cmd/compile: inconsistent nil check elimination on ppc64 #9058

gopherbot opened this issue Nov 3, 2014 · 4 comments
Labels
FrozenDueToAge
Milestone
Unplanned

Comments

@gopherbot
Copy link

by austin@google.com:

What steps will reproduce the problem?
1. GOARCH=power64 go run run.go -- nilptr2.go

Lines marked "removed repeated nil check" fail in this test because the test
depends on duplicate store elimination, which doesn't currently happen on power64x.

To see the cause, run
  GOARCH=power64 gdb --args $(go tool -n 9g) nilptr3.go
  br nilopt
  r
  dis 1
  set debug_checknil = 2
  c

On power64, prior to nilopt, f2 looks like
0:00041 (nilptr3.go:64)        TEXT      "".f2(SB),$0-0
0:00042 (nilptr3.go:64) FUNCDATA    $0,"".gcargs·2(SB)
0:00043 (nilptr3.go:64) FUNCDATA    $1,"".gclocals·3(SB)
0:00052 (nilptr3.go:66) MOVD    $0,R7
0:00053 (nilptr3.go:66) MOVD    R7,"".intp-0(SP)
0:00054 (nilptr3.go:67) MOVD    $0,R7
0:00055 (nilptr3.go:67) MOVD    R7,"".arrayp-0(SP)
0:00056 (nilptr3.go:68) MOVD    $0,R7
0:00057 (nilptr3.go:68) MOVD    R7,"".array0p-0(SP)
0:00058 (nilptr3.go:69) MOVD    $0,R7
0:00059 (nilptr3.go:69) MOVD    R7,"".bigarrayp-0(SP)
0:00060 (nilptr3.go:70) MOVD    $0,R7
0:00061 (nilptr3.go:70) MOVD    R7,"".structp-0(SP)
0:00062 (nilptr3.go:71) MOVD    $0,R7
0:00063 (nilptr3.go:71) MOVD    R7,"".bigstructp-0(SP)
0:00064 (nilptr3.go:72) MOVD    $0,R7
0:00065 (nilptr3.go:72) MOVD    R7,"".emptyp-0(SP)
0:00066 (nilptr3.go:73) MOVD    $0,R7
0:00067 (nilptr3.go:73) MOVD    R7,"".empty1p-0(SP)
0:00068 (nilptr3.go:76) MOVD    "".intp-0(SP),R7
0:00069 (nilptr3.go:76) CHECKNIL    R7,
0:00070 (nilptr3.go:77) MOVD    "".arrayp-0(SP),R7
0:00071 (nilptr3.go:77) CHECKNIL    R7,
0:00072 (nilptr3.go:78) MOVD    "".array0p-0(SP),R7
0:00073 (nilptr3.go:78) CHECKNIL    R7,
0:00074 (nilptr3.go:79) MOVD    "".array0p-0(SP),R7
0:00075 (nilptr3.go:79) CHECKNIL    R7,
0:00076 (nilptr3.go:80) MOVD    "".intp-0(SP),R7
0:00077 (nilptr3.go:80) CHECKNIL    R7,
0:00078 (nilptr3.go:81) MOVD    "".arrayp-0(SP),R7
0:00079 (nilptr3.go:81) CHECKNIL    R7,
0:00080 (nilptr3.go:82) MOVD    "".structp-0(SP),R7
0:00081 (nilptr3.go:82) CHECKNIL    R7,
0:00082 (nilptr3.go:83) MOVD    "".emptyp-0(SP),R7
0:00083 (nilptr3.go:83) CHECKNIL    R7,
0:00084 (nilptr3.go:84) MOVD    "".arrayp-0(SP),R7
0:00085 (nilptr3.go:84) CHECKNIL    R7,
0:00086 (nilptr3.go:85) MOVD    "".bigarrayp-0(SP),R7
0:00087 (nilptr3.go:85) CHECKNIL    R7,
0:00088 (nilptr3.go:86) MOVD    "".bigstructp-0(SP),R7
0:00089 (nilptr3.go:86) CHECKNIL    R7,
0:00090 (nilptr3.go:87) MOVD    "".empty1p-0(SP),R7
0:00091 (nilptr3.go:87) CHECKNIL    R7,
0:00092 (nilptr3.go:88) RETURN  ,

These extra MOVDs inhibit the repeated nil check optimization.  In contrast, on amd64,
prior to nilopt, f2 looks like
0:00032 (nilptr3.go:64) TEXT    "".f2+0(SB),$0-0
0:00033 (nilptr3.go:64) FUNCDATA    $0,"".gcargs·2+0(SB)
0:00034 (nilptr3.go:64) FUNCDATA    $1,"".gclocals·3+0(SB)
0:00043 (nilptr3.go:66) MOVQ    $0,DX
0:00044 (nilptr3.go:67) MOVQ    $0,AX
0:00045 (nilptr3.go:68) MOVQ    $0,CX
0:00046 (nilptr3.go:69) MOVQ    $0,R8
0:00047 (nilptr3.go:70) MOVQ    $0,DI
0:00048 (nilptr3.go:71) MOVQ    $0,SI
0:00049 (nilptr3.go:72) MOVQ    $0,BP
0:00050 (nilptr3.go:73) MOVQ    $0,BX
0:00051 (nilptr3.go:76) CHECKNIL    DX,
0:00052 (nilptr3.go:77) CHECKNIL    AX,
0:00053 (nilptr3.go:78) CHECKNIL    CX,
0:00054 (nilptr3.go:79) CHECKNIL    CX,
0:00055 (nilptr3.go:80) CHECKNIL    DX,
0:00056 (nilptr3.go:81) CHECKNIL    AX,
0:00057 (nilptr3.go:82) CHECKNIL    DI,
0:00058 (nilptr3.go:83) CHECKNIL    BP,
0:00059 (nilptr3.go:84) CHECKNIL    AX,
0:00060 (nilptr3.go:85) CHECKNIL    R8,
0:00061 (nilptr3.go:86) CHECKNIL    SI,
0:00062 (nilptr3.go:87) CHECKNIL    BX,
0:00063 (nilptr3.go:88) RET ,
@aclements
Copy link
Member

This test still fails, even though optimization now works on ppc64, but it fails for a completely different reason. Because R0 is a constant 0, ppc64's peephole optimizer effectively does constant propagation of 0 for free. As a result, after peep this looks like:

0:00041 (nilptr3.go:62)        TEXT      "".f2(SB),$0-0
0:00042 (nilptr3.go:62) FUNCDATA    $0,"".gcargs·2(SB)
0:00043 (nilptr3.go:62) FUNCDATA    $1,"".gclocals·3(SB)
0:00069 (nilptr3.go:74) CHECKNIL    R0,
0:00071 (nilptr3.go:75) CHECKNIL    R0,
0:00073 (nilptr3.go:76) CHECKNIL    R0,
0:00075 (nilptr3.go:77) CHECKNIL    R0,
0:00077 (nilptr3.go:78) CHECKNIL    R0,
0:00079 (nilptr3.go:79) CHECKNIL    R0,
0:00081 (nilptr3.go:80) CHECKNIL    R0,
0:00083 (nilptr3.go:81) CHECKNIL    R0,
0:00085 (nilptr3.go:82) CHECKNIL    R0,
0:00087 (nilptr3.go:83) CHECKNIL    R0,
0:00089 (nilptr3.go:84) CHECKNIL    R0,
0:00091 (nilptr3.go:85) CHECKNIL    R0,
0:00092 (nilptr3.go:86) RETURN  ,

However, the CHECKNILs still don't get eliminated because regtyp(R0) returns 0 (because R0 isn't a normal register), so they aren't even considered for elimination. It's possible regtyp could return 1 for R0, but then it would fail the test in the other direction because ppc64 would eliminate CHECKNILs more effectively than other platforms.

@aclements aclements self-assigned this Dec 10, 2014
@bradfitz bradfitz modified the milestone: Go1.5 Dec 16, 2014
@rsc rsc changed the title test/nilptr2.go: fails on power64x cmd/internal/gc: inconsistent nil check elimination on ppc64 Apr 26, 2015
@rsc rsc modified the milestones: Go1.5Maybe, Go1.5 Apr 26, 2015
@rsc rsc changed the title cmd/internal/gc: inconsistent nil check elimination on ppc64 cmd/compile: inconsistent nil check elimination on ppc64 Jun 8, 2015
@rsc
Copy link
Contributor

rsc commented Jun 29, 2015

Too late for Go 1.5. Maybe SSA will take care of this.

@rsc rsc modified the milestones: Unplanned, Go1.5Maybe Jun 29, 2015
@gopherbot
Copy link
Author

CL https://golang.org/cl/14461 mentions this issue.

minux pushed a commit that referenced this issue Nov 12, 2015
@cherrymui @minux
test: fix nosplit.go, fixedbugs/issue11656.go and skip two tests for …
15676b5 
…mips64{,le}

Skip fixedbugs/issue10607.go because external linking is not supported
yet.

Skip nilptr3.go because of issue #9058 (same as ppc64).

Change-Id: Ib3dfbd9a03ee4052871cf57c74b3cc5e745e1f80
Reviewed-on: https://go-review.googlesource.com/14461
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@cherrymui
Copy link
Member

Closed. SSA takes care of this, and nilptr3.go is gone.

@golang golang locked and limited conversation to collaborators Sep 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@bradfitz @rsc @aclements @gopherbot @cherrymui