Bug 129420 - Incorrect V_JITOperation_EJ call implementation in DFG for 32-bit ports.
Summary: Incorrect V_JITOperation_EJ call implementation in DFG for 32-bit ports.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 108645
  Show dependency tree / graph
 
Reported: 2014-02-27 05:03 PST by Julien Brianceau
Modified: 2014-02-27 10:42 PST (History)
7 users (show)

See Also:

Attachments
Fix V_JITOperation_EJ call implementation in DFG for 32-bit ports. (1.32 KB, patch)
2014-02-27 05:09 PST, Julien Brianceau 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Brianceau 2014-02-27 05:03:12 PST 
r162652 (http://trac.webkit.org/changeset/162652) introduced V_JITOperation_EJ prototype.

Its implementation seems to be incorrect for 32-bit ports.
Comment 1 Julien Brianceau 2014-02-27 05:09:15 PST 
Created attachment 225357 [details]
Fix V_JITOperation_EJ call implementation in DFG for 32-bit ports.
Comment 2 Zoltan Herczeg 2014-02-27 05:23:03 PST 
Was this a speculative fix?
Comment 3 Julien Brianceau 2014-02-27 05:26:05 PST 
(In reply to comment #2)
> Was this a speculative fix?
Yes, I only checked compilation. I'm going to retrieve my env to launch run-layout-jsc tests on my arm traditional board.
Comment 4 Julien Brianceau 2014-02-27 05:48:50 PST 
(In reply to comment #3)
> Yes, I only checked compilation. I'm going to retrieve my env to launch run-layout-jsc tests on my arm traditional board.
run-layout-jsc reports the same results on my board with and without this patch: not better, not worse.
Comment 5 Zoltan Herczeg 2014-02-27 06:02:29 PST 
> run-layout-jsc reports the same results on my board with and without this patch: not better, not worse.

Interesting. So this patch is needed or not? I suspect this code is not triggered.
Comment 6 Julien Brianceau 2014-02-27 06:08:08 PST 
(In reply to comment #5)
> > run-layout-jsc reports the same results on my board with and without this patch: not better, not worse.
> 
> Interesting. So this patch is needed or not? I suspect this code is not triggered.

You're right. If I remove this function, compilation is still ok so it's not even compiled actually :)
What's the best choice, removing it or fixing it in case we'd need it later?
Comment 7 Zoltan Herczeg 2014-02-27 06:16:06 PST 
I think we should trigger it somehow, otherwise we don't know whether the fix is correct :(
Comment 8 Julien Brianceau 2014-02-27 07:40:42 PST 
(In reply to comment #7)
> I think we should trigger it somehow, otherwise we don't know whether the fix is correct :(
As it's like http://trac.webkit.org/changeset/157797 for the tag/payload swap, I'm pretty sure the fix is correct. However if the code is not compiled, I won't be opposed to just remove it.
Comment 9 Yong Li 2014-02-27 08:05:58 PST 
Comment on attachment 225357 [details]
Fix V_JITOperation_EJ call implementation in DFG for 32-bit ports.

Looks right to me
Comment 10 Yong Li 2014-02-27 08:08:46 PST 
Comment on attachment 225357 [details]
Fix V_JITOperation_EJ call implementation in DFG for 32-bit ports.

Withdraw r+. A test case should have helped to find this problem.
Comment 11 Geoffrey Garen 2014-02-27 10:11:20 PST 
Comment on attachment 225357 [details]
Fix V_JITOperation_EJ call implementation in DFG for 32-bit ports.

r=me
Comment 12 WebKit Commit Bot 2014-02-27 10:42:23 PST 
Comment on attachment 225357 [details]
Fix V_JITOperation_EJ call implementation in DFG for 32-bit ports.

Clearing flags on attachment: 225357

Committed r164813: <http://trac.webkit.org/changeset/164813>
Comment 13 WebKit Commit Bot 2014-02-27 10:42:26 PST 
All reviewed patches have been landed.  Closing bug.