Summary: | Avoid to add zero offset in BaseIndex. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | László Langó <llango.u-szeged> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, fpizlo | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
László Langó
2013-12-04 02:50:05 PST
Created attachment 218394 [details]
Patch
Comment on attachment 218394 [details] Patch Clearing flags on attachment: 218394 Committed r160091: <http://trac.webkit.org/changeset/160091> All reviewed patches have been landed. Closing bug. As a general rule, I would avoid making such changes. This change has no upside. The C compiler would have folded the zero, and it would have done it as part of one of many already-mandatory folding passes - even at -O0. So, you're just duplicating that functionality in offlineasm, which just means more code to maintain and more places where someone might make a mistake. (In reply to comment #4) > As a general rule, I would avoid making such changes. This change has no upside. The C compiler would have folded the zero, and it would have done it as part of one of many already-mandatory folding passes - even at -O0. So, you're just duplicating that functionality in offlineasm, which just means more code to maintain and more places where someone might make a mistake. Thanks for the reply. Yes, you are partly right. The GCC with -O0 really optimize this out, but the clang isn't. It won't do it with -O0. The other reason why I made this is that there's already a similar change in class Address in this file: http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/offlineasm/cloop.rb#L195 So we should keep or remove from both places. I'm still think this is a good change. |