Summary: | FTL: split overflow checks into non-overflow arithmetic and an additional call to the overflow intrinsic check. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nadav Rotem <nrotem> | ||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, fpizlo, ggaren | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Nadav Rotem
2013-10-01 12:35:39 PDT
Created attachment 213113 [details]
Patch
Comment on attachment 213113 [details]
Patch
r=me
Does SelectionDAG also catch the multiply case?
Comment on attachment 213113 [details]
Patch
Sorry, one style nit: The "Reviewed by" line needs to be right under the bug name/URL pair. I think some of our automated tools expect to find it there.
Okay. Should I resubmit ? Yes. It works with Mul, Add and Inc. I am sure that we are missing some patterns but we can fix them as we go. This: function foo() { var sum = 1 while (sum < 100000000) sum *= 10 } Becomes this: LBB0_1: ## =>This Inner Loop Header: Depth=1 imull $10, %eax, %ecx jo LBB0_7 ## BB#2: ## %OSR exit continuation for @11<Int32> ## in Loop: Header=BB0_1 Depth=1 testl %ecx, %ecx je LBB0_3 LBB0_4: ## %ArithMul continuation ## in Loop: Header=BB0_1 Depth=1 cmpl $100000000, %ecx ## imm = 0x5F5E100 movl %ecx, %eax jl LBB0_1 jmp LBB0_5 > Okay. Should I resubmit ?
Yes please.
Created attachment 213117 [details]
Patch
Added commit-queue=?. Comment on attachment 213117 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213117&action=review Looks great! > Source/JavaScriptCore/ChangeLog:36 > +LBB0_1: ## %OSR exit continuation for @24<Int32> > + ## =>This Inner Loop Header: Depth=1 > + movl %ecx, %esi > + incl %esi > + jo LBB0_4 I would indent this. Indent the commit message ? In what way ? (In reply to comment #10) > Indent the commit message ? In what way ? The ChangeLog messages should have at least eight-space indent except for the header ("2013-10-01 Nadav Rotem <nrotem@apple.com>"). So this part of your patch: +LBB0_1: ## %OSR exit continuation for @24<Int32> + ## =>This Inner Loop Header: Depth=1 + movl %ecx, %esi + incl %esi + jo LBB0_4 Should be: + LBB0_1: ## %OSR exit continuation for @24<Int32> + ## =>This Inner Loop Header: Depth=1 + movl %ecx, %esi + incl %esi + jo LBB0_4 We have some scripts that try to parse changelog's, so sticking to the format is usually a good idea. Created attachment 213124 [details]
Patch
Comment on attachment 213124 [details] Patch Clearing flags on attachment: 213124 Committed r156746: <http://trac.webkit.org/changeset/156746> All reviewed patches have been landed. Closing bug. |