-
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
cmd/compile: extraneous CMPQ instructions #15509
Comments
Can you explain why the comparison is not necessary? It looks perfectly fine, but I'm jet-lagged so my mind is not very sharp right now. |
The resulting flag doesn't get used, and it gets overwritten by the CMPQ at 00018. |
Ah, I missed that there are three comparisons, not two. It's actually subtle, a copy is introduced during flagalloc pass. The original is dead, but never removed.
|
I wonder if the comparison should have been moved during the tighten pass. v11 has a single use, so it should be defined in b4 instead of b1, right? I'm not suggesting it's bug, just that it's not aggressive enough.
|
Yeah. Tighten won't move any values that depend on two values, for fear of increasing the number of live values that must be spilled and loaded. However, v11 depends on args, which never need to get spilled, so at worst this will (in general) introduce extra loads. There are other kinds of args (constants, what else?) that probably also should not get counted when making this decision. |
Fixing tighten to ignore args and rematerializeable values sounds good (for 1.8). |
Ok. Added to my queue. I don't think it'll fix all instances of this problem, but I will find out. :) |
CL https://golang.org/cl/28390 mentions this issue. |
Result:
Observe that the CMPQ at 00010 is unnecessary. This arose when working on optimizing scanobject.
cc @randall77 @brtzsnr @aclements
The text was updated successfully, but these errors were encountered: