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.

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

cmd/asm: change canonical spelling of CMOVLEQ to CMOVL.EQ etc #20173

Open
dlespiau opened this Issue Apr 29, 2017 · 6 comments

Comments

Projects
None yet
5 participants
@dlespiau
Contributor

dlespiau commented Apr 29, 2017

This proposal is originally from @rsc in issue #14069 and is about CMOVLEQ being ambiguous. I Extracted it as I believe it deserves its own issue.

Recounting this to @aclements I figured out what is going on with CMOVLEQ. The GNU form is CMOV[cond][size] but the form added for Go is CMOV[size][cond]. So GNU's cmovleq is cmov-le-q while Go's CMOVLEQ is CMOV-L-EQ. The Intel syntax has no size suffix and is CMOVEQ/CMOVLE, but it seems clear that Go should not be inserting a size suffix in the middle of the defined opcode name. While in general I feel that we need to put up with past mistakes in our assembly definitions to avoid breaking existing assembly programs, this one seems so egregious and error-prone that I think we have no choice but to change these names for Go 1.7 to match the Intel and GNU syntax.

Unfortunately it seems too late for such a breaking change so the proposal evolved into introducing new aliases like CMOVL.EQ (CMOV%size.%cond) to lift the ambiguity and then, later on, remove the old versions. Maybe.

@gopherbot gopherbot added this to the Proposal milestone Apr 29, 2017

@gopherbot gopherbot added the Proposal label Apr 29, 2017

@gopherbot

This comment has been minimized.

gopherbot commented Apr 29, 2017

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

@rsc

This comment has been minimized.

Contributor

rsc commented May 1, 2017

At this point I don't think we can rename instructions like this - it will silently change the meaning of existing (one assumes debugged) code. If we need to do something at all, I would suggest adding CMOVL.EQ as an alias for current Go CMOVLEQ, and so on, and then in a later release removing the dot-free variants.

@rsc rsc changed the title from Proposal: swap [size] and [cond] in CMOV[size][cond] to cmd/asm: change canonical spelling of CMOVLEQ to CMOVL.EQ etc Jun 12, 2017

@rsc

This comment has been minimized.

Contributor

rsc commented Jun 12, 2017

No one replied to previous comment (but two thumbs up) so I'm repurposing for that "clearer new spelling" instead of "change existing meaning" fix.

@rsc rsc modified the milestones: Go1.10, Proposal Jun 12, 2017

@dlespiau

This comment has been minimized.

Contributor

dlespiau commented Jun 15, 2017

Yes, thank you for renaming the issue. Updated the proposal text accordingly as well.

@gopherbot

This comment has been minimized.

gopherbot commented Sep 27, 2017

Change https://golang.org/cl/66451 mentions this issue: cmd/asm: add CMOVL.EQ->CMOVLEQ alias

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017

@Quasilyte

This comment has been minimized.

Contributor

Quasilyte commented May 18, 2018

Should this be done for 1.11?

Suffixes are used for AVX-512 things, should we reconsider CMOVL.EQ or add additional suffixes for these? (see #22779)

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jun 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment