Summary: | DFG non-speculative JIT does not reuse registers when compiling comparisons | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | barraclough, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Filip Pizlo
2011-06-28 14:42:24 PDT
Created attachment 98975 [details]
the patch
Comment on attachment 98975 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=98975&action=review r- based on above comments, I think we can simplify further by switching to the 3-op branchAdd32 etc calls. In basicArithOp we're also retagging arg2 in the case of overflow (& a register has been reused), which I think we can remove if we stop zero-extending the operands. > Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp:-176 > - Actually, I think we can just remove this - the macro assembler has a 3-op form: branchAdd32(ResultCondition cond, RegisterID src, TrustedImm32 imm, RegisterID dest) (& branchSub32). These should be more helpful for platforms with 3-op instructions, like arm. BTW, if we didn't have the 3-op branchAdd32, then I don't think you need zero extension here (since 32bit ops ignore the high bits coming in, and always set high bits to zero), so I think you could just use a move. And the macro assembler will automatically elide moves to the same register, so you wouldn't need the if check. > Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp:291 > m_jit.zeroExtend32ToPtr(arg1GPR, resultGPR); Looking over this, again, I don't think we need the zero extends here, and as such, I don't think we need use to use a temporary, at minimum in the case of no overflow. Again, there are 3-op branchAdd/Sub/Mul calls, which should allow us to plant this in a single instruction on arm. Created attachment 99019 [details]
the patch (fix review)
Comment on attachment 99019 [details] the patch (fix review) Clearing flags on attachment: 99019 Committed r90189: <http://trac.webkit.org/changeset/90189> All reviewed patches have been landed. Closing bug. |