Summary: | LLInt ARM backend should not use the d8 register as scratch register | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gabor Rapcsanyi <rgabor> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | barraclough, benjamin, commit-queue, fpizlo, oliver, ossy, sg5.lee, zherczeg | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 108645 | ||||||
Attachments: |
|
Description
Gabor Rapcsanyi
2013-04-18 06:40:12 PDT
Created attachment 198731 [details]
proposed fix
I don't know much about LLInt, but why does the ABI matters at all here? I would think the only thing that matters is follow the JIT conventions(?). (In reply to comment #2) > I don't know much about LLInt, but why does the ABI matters at all here? > I would think the only thing that matters is follow the JIT conventions(?). I think that the LLInt uses more scratch registers, and it doesn't have to use the same scratch registers as the JIT. So this change looks valid to me. Comment on attachment 198731 [details] proposed fix Clearing flags on attachment: 198731 Committed r148705: <http://trac.webkit.org/changeset/148705> All reviewed patches have been landed. Closing bug. Is there any reason to duplicate my preceding bug report and fix (https://bugs.webkit.org/show_bug.cgi?id=114495) as new one? (In reply to comment #6) > Is there any reason to duplicate my preceding bug report and fix (https://bugs.webkit.org/show_bug.cgi?id=114495) as new one? In WebKit, we care about: -bug report with the best patch. -bug report with the best info. The order in which bugs are reported does not matter. We dupe backward or forward to the best bug report. (In reply to comment #6) > Is there any reason to duplicate my preceding bug report and fix (https://bugs.webkit.org/show_bug.cgi?id=114495) as new one? Sorry, I forgot to check before I reported and fixed it. I found this bug after debugging a bug for a week. Hello, Benjamin Poulain > In WebKit, we care about: > -bug report with the best patch. > -bug report with the best info. > The order in which bugs are reported does not matter. We dupe backward or > forward to the best bug report. What do you mean by "best patch" or "best info"? Do you mean followings? 1) I didn't make my suggested patch as attached patch. 2) I didn't request to reviewer more aggressively (by using IRC or by sending mail again and again) I cannot find the difference between mine and this one: title(114495-mine): LLInt should not use d8 register as scratch register title(114811-this): LLInt ARM backend should not use the d8 register as scratch register and here is my bug report: Currently, LLInt uses d8 register as scratch register as followings: ARM_SCRATCH_FPR = SpecialRegister.new("d8") in arm.rb C_LOOP_SCRATCH_FPR = SpecialRegister.new("d8") in cloop.rb However, AAPCS §5.1.2.1 says Registers s16-s31 (d8-d15, q4-q7) must be preserved across subroutine calls; registers s0-s15 (d0-d7, q0-q3) do not need to be preserved (and can be used for passing arguments or returning results in standard procedure-call variants). Registers d16-d31 (q8-q15), if present, do not need to be preserved. Therefore it should not use d8 for that purpose. I think it would be safe to use d6. (Register d7 is already in use for other purpose. ) ARM_SCRATCH_FPR = SpecialRegister.new("d6") in arm.rb C_LOOP_SCRATCH_FPR = SpecialRegister.new("d6") in cloop.rb Gábor ran into the same bug and then reported and fixed it quickly. And then apologized to you because he didn't check/find if there was already a bug report about this bug. Additionally it isn't so easy to find it, because there are 15847 unconfirmed/new bugs now. If you file a new bug report without cc-ing/poke-ing folks maybe nobody will notice that you filed a new bug report. *** Bug 114495 has been marked as a duplicate of this bug. *** |