Bug 154904 - [JSC] Simplify ArithMod(ArithMod(x, const1), const2) if const2 >= const1
Summary: [JSC] Simplify ArithMod(ArithMod(x, const1), const2) if const2 >= const1
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-01 21:07 PST by Benjamin Poulain
Modified: 2016-03-01 23:53 PST (History)
5 users (show)

See Also:


Attachments
Patch (4.49 KB, patch)
2016-03-01 21:09 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch for landing (4.47 KB, patch)
2016-03-01 22:57 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2016-03-01 21:07:14 PST
[JSC] Simplify ArithMod(ArithMod(x, const1), const2) if const2 >= const1
Comment 1 Benjamin Poulain 2016-03-01 21:09:27 PST
Created attachment 272630 [details]
Patch
Comment 2 Saam Barati 2016-03-01 22:15:08 PST
Comment on attachment 272630 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=272630&action=review

LGTM

> Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:179
> +            // Out: Identity(ArithMod(x, const1)

Style nit: parens aren't balanced

> Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:184
> +                && m_node->child1()->binaryUseKind()

Should this also be == Int32Use?
Comment 3 Benjamin Poulain 2016-03-01 22:16:15 PST
(In reply to comment #2)
> Comment on attachment 272630 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272630&action=review
> 
> LGTM
> 
> > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:179
> > +            // Out: Identity(ArithMod(x, const1)
> 
> Style nit: parens aren't balanced
> 
> > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:184
> > +                && m_node->child1()->binaryUseKind()
> 
> Should this also be == Int32Use?

Yep, indeed!
Comment 4 Benjamin Poulain 2016-03-01 22:57:18 PST
Created attachment 272637 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2016-03-01 23:53:12 PST
Comment on attachment 272637 [details]
Patch for landing

Clearing flags on attachment: 272637

Committed r197445: <http://trac.webkit.org/changeset/197445>
Comment 6 WebKit Commit Bot 2016-03-01 23:53:16 PST
All reviewed patches have been landed.  Closing bug.