| Summary: | MacroAssemblerARM should use xor to swap registers instead of move | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gabor Rapcsanyi <rgabor> | ||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | benjamin, commit-queue, fpizlo, ossy, scerveau, zherczeg | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 108645 | ||||||
| Attachments: |
|
||||||
Created attachment 202065 [details]
proposed fix
Maybe we should consider to make this change on ARMv7 as well.
Comment on attachment 202065 [details]
proposed fix
r=me
Comment on attachment 202065 [details] proposed fix Clearing flags on attachment: 202065 Committed r150748: <http://trac.webkit.org/changeset/150748> All reviewed patches have been landed. Closing bug. (In reply to comment #1) > Maybe we should consider to make this change on ARMv7 as well. I think we should not. ARMv7 has register aliasing, making "mov" essentially free in most situation. Does the bug exist on ARMv7 though? Well, let's say, some ARMv7 have register aliasing... :) (In reply to comment #5) > (In reply to comment #1) > > Maybe we should consider to make this change on ARMv7 as well. > > I think we should not. > ARMv7 has register aliasing, making "mov" essentially free in most situation. > > Does the bug exist on ARMv7 though? Yeah this doesn't feel right for the ARMv7 assembler. There we have scratch registers for doing this. So long as there is a spare register the moves will be faster. *** Bug 112697 has been marked as a duplicate of this bug. *** This bug also fixed many failing tests reported in https://bugs.webkit.org/show_bug.cgi?id=105304 *** Bug 113774 has been marked as a duplicate of this bug. *** *** Bug 117349 has been marked as a duplicate of this bug. *** |
On ARM Traditional and Thumb2 we are using a temporary register to swap two register value. ARM Traditional using the r3 register for that purpose. MacroAssemblerARM.h:540 void swap(RegisterID reg1, RegisterID reg2) { move(reg1, ARMRegisters::S0); move(reg2, reg1); move(ARMRegisters::S0, reg2); } That causing a problem because there is a case when we set up the function parameters in DFGCCallHelpers.h we put the third parameter to r3 then in setupTwoStubArgs() we swap r1 and r2 using r3 as temporary register. DFGCCallHelpers.h:460 if (arg1 != GPRInfo::argumentGPR3 && arg2 != GPRInfo::argumentGPR3) { move(arg3, GPRInfo::argumentGPR3); setupTwoStubArgs<GPRInfo::argumentGPR1, GPRInfo::argumentGPR2>(arg1, arg2); return; } This generates the following code which is not correct: => 0x40022a3c: mov r3, r4 => 0x40022a40: mov r3, r1 => 0x40022a44: mov r1, r2 => 0x40022a48: mov r2, r3 ARM Thumb2 is using the r12 register so this bug doesn't affect it.