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.

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 · 8 comments
Open

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

dlespiau opened this issue Apr 29, 2017 · 8 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Milestone

Comments

@dlespiau
Copy link
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
Copy link

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

@rsc
Copy link
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 Proposal: swap [size] and [cond] in CMOV[size][cond] cmd/asm: change canonical spelling of CMOVLEQ to CMOVL.EQ etc Jun 12, 2017
@rsc
Copy link
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
Copy link
Contributor Author

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

@gopherbot
Copy link

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
Copy link
Contributor

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
@ianlancetaylor ianlancetaylor modified the milestones: Go1.12, Unplanned Nov 28, 2018
@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Nov 28, 2018
@gopherbot
Copy link

Change https://golang.org/cl/171732 mentions this issue: cmd/internal/obj/x86: permit new CMOVL syntax with suffix

@Symmtek
Copy link

Symmtek commented Apr 13, 2019

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.

Added an alias CMOVL.EQ for CMOVLEQ. Should probably remove old version in the future.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

6 participants