B3 should not give ValueReps for the non-stackmap children of a CheckValue to the...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Dec 2015 20:25:30 +0000 (20:25 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Dec 2015 20:25:30 +0000 (20:25 +0000)
commita3ad4eebd390931295ce086b604e57bc718763d0
treec390f28d64501cc86a24a911b3d4bd53b5d3fbdc
parent35ec90801328a2b6e31e0909e8107da3edbcb0fe
B3 should not give ValueReps for the non-stackmap children of a CheckValue to the generator callback
https://bugs.webkit.org/show_bug.cgi?id=152224

Reviewed by Geoffrey Garen.

Previously, a stackmap generator for a Check had to know how many children the B3 value for the
Check had at the time of code generation. That meant that B3 could not change the kind of Check
that it was - for example it cannot turn a Check into a Patchpoint and it cannot turn a CheckAdd
into a Check. But just changing the contract so that the stackmap generation params only get the
stackmap children of the check means that B3 can transform Checks as it likes.

This is meant to aid sinking values into checks.

Also, I found that the effects of a Check did not include HeapRange::top(). I think it's best if
exitsSideways does not imply reading top, the way that it does in DFG. In the DFG, that makes
sense because the exit analysis is orthogonal, so the clobber analysis tells you about the reads
not counting OSR exit - if you need to you can conditionally merge that with World based on a
separate exit analysis. But in B3, the Effects object tells you about both exiting and reading,
and it's computed by one analysis. Prior to this change, Check was not setting reads to top() so
we were effectively saying that Effects::reads is meaningless when exitsSideways is true. It
seems more sensible to instead force the analysis to set reads to top() when setting
exitsSideways to true, not least because we only have one such analysis and many users. But it
also makes sense for another reason: it allows us to bound the set of things that the program
will read after it exits. That might not be useful to us now, but it's a nice feature to get for
free. I've seen language features that have behave like exitsSideways that don't also read top,
like an array bounds check that causes sudden termination without making any promises about how
pretty the crash dump will look.

* b3/B3CheckSpecial.cpp:
(JSC::B3::CheckSpecial::generate):
* b3/B3Opcode.h:
* b3/B3Value.cpp:
(JSC::B3::Value::effects):
* b3/testb3.cpp:
(JSC::B3::testSimpleCheck):
(JSC::B3::testCheckLessThan):
(JSC::B3::testCheckMegaCombo):
(JSC::B3::testCheckAddImm):
(JSC::B3::testCheckAddImmCommute):
(JSC::B3::testCheckAddImmSomeRegister):
(JSC::B3::testCheckAdd):
(JSC::B3::testCheckAdd64):
(JSC::B3::testCheckSubImm):
(JSC::B3::testCheckSubBadImm):
(JSC::B3::testCheckSub):
(JSC::B3::testCheckSub64):
(JSC::B3::testCheckNeg):
(JSC::B3::testCheckNeg64):
(JSC::B3::testCheckMul):
(JSC::B3::testCheckMulMemory):
(JSC::B3::testCheckMul2):
(JSC::B3::testCheckMul64):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::blessSpeculation):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@194048 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/B3CheckSpecial.cpp
Source/JavaScriptCore/b3/B3Effects.h
Source/JavaScriptCore/b3/B3Opcode.h
Source/JavaScriptCore/b3/B3Value.cpp
Source/JavaScriptCore/b3/testb3.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp