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: remove spadj NOPs? #16057

Closed
josharian opened this issue Jun 13, 2016 · 2 comments
Closed

cmd/compile: remove spadj NOPs? #16057

josharian opened this issue Jun 13, 2016 · 2 comments
Labels
FrozenDueToAge
Milestone
Go1.8

Comments

@josharian
Copy link
Contributor

cmd/internal/obj/x86/obj6.go contains this comment and code for when a function requires no stack space:

    if autoffset != 0 {
        // requires stack space...
    } else {
        // zero-byte stack adjustment.
        // Insert a fake non-zero adjustment so that stkcheck can
        // recognize the end of the stack-splitting prolog.
        p = obj.Appendp(ctxt, p)

        p.As = obj.ANOP
        p.Spadj = int32(-ctxt.Arch.PtrSize)
        p = obj.Appendp(ctxt, p)
        p.As = obj.ANOP
        p.Spadj = int32(ctxt.Arch.PtrSize)
    }

Looking through stkcheck, I don't see where/how this matters. If you remove the else branch, all.bash still passes. Do we still need to insert these NOPs?

@randall77 @mwhudson @rsc
@josharian josharian added this to the Go1.8 milestone Jun 13, 2016
@mwhudson
Copy link
Contributor

This must be cruft from before instruction selection was done in the compiler, the ANOPs translate to nothing at all in the machine code so there's nothing for stkcheck to find. Delete it! (in 1.8)

@josharian josharian self-assigned this Jun 14, 2016
josharian added a commit to josharian/go that referenced this issue Jun 16, 2016
@josharian
cmd/internal/obj/x86: remove pointless NOPs
a0cd318 
They are no longer needed by stkcheck.

Fixes golang#16057

Change-Id: I57cb55de5b7a7a1d31a3da200a3a2d51576b68f5
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Aug 25, 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
Go1.8
Development

No branches or pull requests
3 participants
@josharian @mwhudson @gopherbot