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

strings: fix indexShortStr for 17+ bytes #15689

Closed
randall77 opened this issue May 15, 2016 · 4 comments
Closed

strings: fix indexShortStr for 17+ bytes #15689

randall77 opened this issue May 15, 2016 · 4 comments
Labels
FrozenDueToAge Performance
Milestone
Go1.8

Comments

@randall77
Copy link
Contributor

There's a bug in indexShortStr for 17+ bytes. That case has been disabled for the 1.7 release. This bug is to track the fix and reenabling of this case for 1.8.
See issue #15679 .

@TocarIP
@randall77 randall77 added this to the Go1.8 milestone May 15, 2016
@randall77
Copy link
Contributor Author

I think maybe the fix is simple. The code mistakenly uses the 16-byte code when there are 17 bytes.

_16_or_more:
    CMPQ AX, $17
    JA   _17_to_31

The fix may be a simple as using JAE instead.
But I want some more tests before reenabling.

@minux
Copy link
Member

minux commented May 17, 2016 via email

@TocarIP
Copy link
Contributor

TocarIP commented May 17, 2016 •
edited

This is caused by

-       CMPQ BX, $16
+       CMPQ AX, $17

from 6b02a19

Changing 17 back to 16 indeed works.

@gopherbot
Copy link

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

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

No branches or pull requests
4 participants
@minux @randall77 @gopherbot @TocarIP