| Summary: | Support emit_op_mod() for MIPS | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chao-ying Fu <fu> | ||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | barraclough, commit-queue, kent.hansen, zherczeg | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Other | ||||||||
| OS: | Linux | ||||||||
| Attachments: |
|
||||||||
|
Description
Chao-ying Fu
2010-07-22 16:14:24 PDT
Created attachment 62358 [details]
Use div for MIPS op_mod
The patch replaces X86Registers::eax/ecx/edx with regT0/regT2/regT1, so that MIPS can reuse source code to implement op_mod. The MIPS build was ok, while I didn't test x86 build. Thanks!
(In reply to comment #1) > Created an attachment (id=62358) [details] > Use div for MIPS op_mod > > The patch replaces X86Registers::eax/ecx/edx with regT0/regT2/regT1, so that MIPS can reuse source code to implement op_mod. The MIPS build was ok, while I didn't test x86 build. Thanks! For X86 I'd ASSERT(regT0 == eax), etc., since the registers are implicit in the div instruction. It's important that this information is preserved, e.g. in case someone decides to change regT0 to r13 on x64. :) (In reply to comment #2) > (In reply to comment #1) > > Created an attachment (id=62358) [details] [details] > > Use div for MIPS op_mod > > > > The patch replaces X86Registers::eax/ecx/edx with regT0/regT2/regT1, so that MIPS can reuse source code to implement op_mod. The MIPS build was ok, while I didn't test x86 build. Thanks! > > For X86 I'd ASSERT(regT0 == eax), etc., since the registers are implicit in the div instruction. It's important that this information is preserved, e.g. in case someone decides to change regT0 to r13 on x64. :) Yes. You are right. I will update the patch to have ASSERT() for X86 and X86_64. Thanks! Regards, Chao-ying Created attachment 62470 [details]
Assert x86 registers
Add ASSERT() to check x86 registers for x86 IDIV instructions.
I think this is a nice cleanup. Although the whole code should be moved to their respective MacroAssembler header files, and refactor this code to be platform independent (This happened with other math functions as well, like the floating point division). Comment on attachment 62470 [details]
Assert x86 registers
r=me
Comment on attachment 62470 [details] Assert x86 registers Rejecting patch 62470 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1 Last 500 characters of output: mpiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Skipped list contained 'media/video-source-error.html ', but no file of that name could be found Testing 20843 test cases. http/tests/security/xssAuditor/object-embed-tag.html -> failed Exiting early after 1 failures. 20546 tests run. 530.92s total testing time 20545 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 28 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/3835017 (In reply to comment #7) > (From update of attachment 62470 [details]) > Rejecting patch 62470 from commit-queue. > > Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1 > Last 500 characters of output: > mpiling Java tests > make: Nothing to be done for `default'. > Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests > Skipped list contained 'media/video-source-error.html ', but no file of that name could be found > Testing 20843 test cases. > http/tests/security/xssAuditor/object-embed-tag.html -> failed > > Exiting early after 1 failures. 20546 tests run. > 530.92s total testing time > > 20545 test cases (99%) succeeded > 1 test case (<1%) had incorrect layout > 28 test cases (<1%) had stderr output > > Full output: http://queues.webkit.org/results/3835017 Hi, Does anyone know why the test failed after applying the patch? Thanks! Regards, Chao-ying Hey Chao-ying, This test was failing intermittently at the end of last week, I can land this patch for you. cheers, G. fixed in r66524 |