Bug 125215 - Avoid to add zero offset in BaseIndex.
Summary: Avoid to add zero offset in BaseIndex.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-04 02:50 PST by László Langó
Modified: 2013-12-05 01:08 PST (History)
2 users (show)

See Also:


Attachments
Patch (1.51 KB, patch)
2013-12-04 02:53 PST, László Langó
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description László Langó 2013-12-04 02:50:05 PST
Avoid to add zero offset in BaseIndex class in offlineasm/cloop.rb.
Comment 1 László Langó 2013-12-04 02:53:00 PST
Created attachment 218394 [details]
Patch
Comment 2 WebKit Commit Bot 2013-12-04 06:39:23 PST
Comment on attachment 218394 [details]
Patch

Clearing flags on attachment: 218394

Committed r160091: <http://trac.webkit.org/changeset/160091>
Comment 3 WebKit Commit Bot 2013-12-04 06:39:25 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Filip Pizlo 2013-12-04 09:14:08 PST
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.
Comment 5 László Langó 2013-12-05 01:08:57 PST
(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.