Bug 84746

Summary: DFG performs incorrect DCE on (some?) intrinsics
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: oliver, ossy
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch oliver: review+

Oliver Hunt
Reported 2012-04-24 11:59:59 PDT
Take this beautiful piece of code: function f(a,b,c) { if (a) Math.abs(b); // or Math.max(b,c) } var o = {valueOf:function(){return print("Working");}} var i = 0 var forceDFGCompile = true; for (; forceDFGCompile&&i < 10000; i++) if (i & 1) f(true, 5.5, 5.6); else f(false, o, i); f(true, o,i); This should output "Working", but does not if the DFG is enabled. This seems to happen to some extent with all the intrinsics. If you set forceDFGCompile to false, it works as expected.
Attachments
the patch (9.86 KB, patch)
2012-04-24 12:36 PDT, Filip Pizlo
oliver: review+
Oliver Hunt
Comment 1 2012-04-24 12:02:30 PDT
Oliver Hunt
Comment 2 2012-04-24 12:11:47 PDT
Returning the result of the intrinsic produces the correct result, so i think this is a DCE bug.
Filip Pizlo
Comment 3 2012-04-24 12:36:27 PDT
Created attachment 138621 [details] the patch
Filip Pizlo
Comment 4 2012-04-24 12:36:50 PDT
Working on hacking up some layout tests. Can I get an RS for those too?
Oliver Hunt
Comment 5 2012-04-24 12:40:48 PDT
Comment on attachment 138621 [details] the patch r=me, rs=me for tests.
Filip Pizlo
Comment 6 2012-04-24 13:43:39 PDT
Csaba Osztrogonác
Comment 7 2012-04-24 13:58:03 PDT
Reopen, because it broke the Qt build: /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp: In member function ‘void JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node&)’: /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2058: error: ‘isPredictedNumber’ was not declared in this scope
Filip Pizlo
Comment 8 2012-04-24 13:58:57 PDT
(In reply to comment #7) > Reopen, because it broke the Qt build: > /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp: In member function ‘void JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node&)’: > /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2058: error: ‘isPredictedNumber’ was not declared in this scope Oh noes! Looking at this now.
Filip Pizlo
Comment 9 2012-04-24 13:59:53 PDT
(In reply to comment #8) > (In reply to comment #7) > > Reopen, because it broke the Qt build: > > /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp: In member function ‘void JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node&)’: > > /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2058: error: ‘isPredictedNumber’ was not declared in this scope > > Oh noes! Looking at this now. Oh, it's a stupid typo. Fix on the way.
Filip Pizlo
Comment 10 2012-04-24 14:01:40 PDT
Build fix for 32-bit landed in http://trac.webkit.org/changeset/115105
Note You need to log in before you can comment on or make changes to this bug.