-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
math/big: different subVV result on pure Go vs assembly implementations #41401
Comments
@josharian I see you made the corresponding changes in CL164966. How do you think? |
Does this result in any observable changes when using only the public API? IIRC it is the responsibility of the caller to ensure that the lengths are equal. It is possible we should document that more clearly. I would love to delete the assembly, but the performance of the Go code is not good enough yet to do that. I got it closer a cycle or two ago, but there's a ways to go. |
@josharian It does not seem to affect the use of public APIs, and may only cause some troubles for people who want to do things in the math library : ( . It seems that the check on the length of x and y in the function should be removed and then noted in the comment. |
Should I close this issue or keep it? |
I believe that the length checks in the Go code are there to prevent bounds checks in the loop; removing them will make things worse. (Feel free to double-check, though.) More docs and comments around this would be welcome, to avoid future confusion, if you are so inclined. |
Personally I think we can close this issue. |
Look at this test:
c1 and c2 will get different results. The reason is that the length of x, y, z is not checked in the assembly code. Some other functions in arith.go have similar problems.
Three solutions:
(1) Remove the length check of x and y in subVV.
(2) Add length check in arith_$GOARCH.s files, though efficiency will become lower.
(3) Remove assembly functions and let the compiler consider these things.
The text was updated successfully, but these errors were encountered: