Bug 84746 - DFG performs incorrect DCE on (some?) intrinsics
Summary: DFG performs incorrect DCE on (some?) intrinsics
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-04-24 11:59 PDT by Oliver Hunt
Modified: 2012-04-24 14:01 PDT (History)
2 users (show)

See Also:


Attachments
the patch (9.86 KB, patch)
2012-04-24 12:36 PDT, Filip Pizlo
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 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.
Comment 1 Oliver Hunt 2012-04-24 12:02:30 PDT
<rdar://problem/11310772>
Comment 2 Oliver Hunt 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.
Comment 3 Filip Pizlo 2012-04-24 12:36:27 PDT
Created attachment 138621 [details]
the patch
Comment 4 Filip Pizlo 2012-04-24 12:36:50 PDT
Working on hacking up some layout tests. Can I get an RS for those too?
Comment 5 Oliver Hunt 2012-04-24 12:40:48 PDT
Comment on attachment 138621 [details]
the patch

r=me, rs=me for tests.
Comment 6 Filip Pizlo 2012-04-24 13:43:39 PDT
Landed in http://trac.webkit.org/changeset/115103
Comment 7 Csaba Osztrogonác 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
Comment 8 Filip Pizlo 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.
Comment 9 Filip Pizlo 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.
Comment 10 Filip Pizlo 2012-04-24 14:01:40 PDT
Build fix for 32-bit landed in http://trac.webkit.org/changeset/115105