- Revision
- 192035
- Author
- fpi...@apple.com
- Date
- 2015-11-04 14:15:00 -0800 (Wed, 04 Nov 2015)
Log Message
B3 should be able to compile a Check
https://bugs.webkit.org/show_bug.cgi?id=150878
Reviewed by Saam Barati.
The Check opcode in B3 is going to be our main OSR exit mechanism. It is a stackmap
value, so you can pass it any number of additional arguments, and you will get to find
out how those arguments are represented at the point that the value lands in the machine
code. Unlike a Patchpoint, a Check branches on a value, with the goal of supporting full
compare/branch fusion. The stackmap's generator runs in an out-of-line path to which
that branch is linked.
This change fills in the glue necessary to compile a Check and it includes a simple
test of this functionality. That test also happens to check that such simple code will
never use callee-saves, which I think is sensible.
* b3/B3LowerToAir.cpp:
(JSC::B3::Air::LowerToAir::append):
(JSC::B3::Air::LowerToAir::ensureSpecial):
(JSC::B3::Air::LowerToAir::fillStackmap):
(JSC::B3::Air::LowerToAir::tryStackSlot):
(JSC::B3::Air::LowerToAir::tryPatchpoint):
(JSC::B3::Air::LowerToAir::tryCheck):
(JSC::B3::Air::LowerToAir::tryUpsilon):
* b3/B3LoweringMatcher.patterns:
* b3/testb3.cpp:
(JSC::B3::testSimplePatchpoint):
(JSC::B3::testSimpleCheck):
(JSC::B3::run):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (192034 => 192035)
--- trunk/Source/_javascript_Core/ChangeLog 2015-11-04 21:46:10 UTC (rev 192034)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-11-04 22:15:00 UTC (rev 192035)
@@ -1,3 +1,35 @@
+2015-11-03 Filip Pizlo <fpi...@apple.com>
+
+ B3 should be able to compile a Check
+ https://bugs.webkit.org/show_bug.cgi?id=150878
+
+ Reviewed by Saam Barati.
+
+ The Check opcode in B3 is going to be our main OSR exit mechanism. It is a stackmap
+ value, so you can pass it any number of additional arguments, and you will get to find
+ out how those arguments are represented at the point that the value lands in the machine
+ code. Unlike a Patchpoint, a Check branches on a value, with the goal of supporting full
+ compare/branch fusion. The stackmap's generator runs in an out-of-line path to which
+ that branch is linked.
+
+ This change fills in the glue necessary to compile a Check and it includes a simple
+ test of this functionality. That test also happens to check that such simple code will
+ never use callee-saves, which I think is sensible.
+
+ * b3/B3LowerToAir.cpp:
+ (JSC::B3::Air::LowerToAir::append):
+ (JSC::B3::Air::LowerToAir::ensureSpecial):
+ (JSC::B3::Air::LowerToAir::fillStackmap):
+ (JSC::B3::Air::LowerToAir::tryStackSlot):
+ (JSC::B3::Air::LowerToAir::tryPatchpoint):
+ (JSC::B3::Air::LowerToAir::tryCheck):
+ (JSC::B3::Air::LowerToAir::tryUpsilon):
+ * b3/B3LoweringMatcher.patterns:
+ * b3/testb3.cpp:
+ (JSC::B3::testSimplePatchpoint):
+ (JSC::B3::testSimpleCheck):
+ (JSC::B3::run):
+
2015-10-30 Keith Miller <keith_mil...@apple.com>
Fix endless OSR exits when creating a rope that contains an object that ToPrimitive's to a number.
Modified: trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp (192034 => 192035)
--- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2015-11-04 21:46:10 UTC (rev 192034)
+++ trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2015-11-04 22:15:00 UTC (rev 192035)
@@ -35,6 +35,7 @@
#include "B3AddressMatcher.h"
#include "B3ArgumentRegValue.h"
#include "B3BasicBlockInlines.h"
+#include "B3CheckSpecial.h"
#include "B3Commutativity.h"
#include "B3IndexMap.h"
#include "B3IndexSet.h"
@@ -462,12 +463,44 @@
insts.last().append(Inst(opcode, currentValue, std::forward<Arguments>(arguments)...));
}
- template<typename T>
- void ensureSpecial(T*& field)
+ template<typename T, typename... Arguments>
+ T* ensureSpecial(T*& field, Arguments&&... arguments)
{
- if (!field)
- field = static_cast<T*>(code.addSpecial(std::make_unique<T>()));
+ if (!field) {
+ field = static_cast<T*>(
+ code.addSpecial(std::make_unique<T>(std::forward<Arguments>(arguments)...)));
+ }
+ return field;
}
+
+ void fillStackmap(Inst& inst, StackmapValue* stackmap, unsigned numSkipped)
+ {
+ for (unsigned i = numSkipped; i < stackmap->numChildren(); ++i) {
+ ConstrainedValue value = stackmap->constrainedChild(i);
+
+ Arg arg;
+ switch (value.rep().kind()) {
+ case ValueRep::Any:
+ arg = immOrTmp(value.value());
+ break;
+ case ValueRep::SomeRegister:
+ arg = tmp(value.value());
+ break;
+ case ValueRep::Register:
+ arg = Tmp(value.rep().reg());
+ append(Move, immOrTmp(value.value()), arg);
+ break;
+ case ValueRep::StackArgument:
+ arg = Arg::callArg(value.rep().offsetFromSP());
+ appendStore(value.value(), arg);
+ break;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ break;
+ }
+ inst.args.append(arg);
+ }
+ }
IndexSet<Value> locked; // These are values that will have no Tmp in Air.
IndexMap<Value, Tmp> valueToTmp; // These are values that must have a Tmp in Air. We say that a Value* with a non-null Tmp is "pinned".
@@ -483,8 +516,6 @@
unsigned currentIndex;
Value* currentValue;
- PatchpointSpecial* patchpointSpecial { 0 };
-
// The address selector will match any pattern where the input operands are available as Tmps.
// It doesn't care about sharing. It will happily emit the same address _expression_ over and over
// again regardless of the _expression_'s complexity. This works out fine, since at the machine
@@ -837,6 +868,7 @@
return true;
}
+ PatchpointSpecial* patchpointSpecial { nullptr };
bool tryPatchpoint()
{
PatchpointValue* patchpointValue = currentValue->as<PatchpointValue>();
@@ -847,34 +879,47 @@
if (patchpointValue->type() != Void)
inst.args.append(tmp(patchpointValue));
- for (ConstrainedValue value : patchpointValue->constrainedChildren()) {
- Arg arg;
- switch (value.rep().kind()) {
- case ValueRep::Any:
- arg = immOrTmp(value.value());
- break;
- case ValueRep::SomeRegister:
- arg = tmp(value.value());
- break;
- case ValueRep::Register:
- arg = Tmp(value.rep().reg());
- append(Move, immOrTmp(value.value()), arg);
- break;
- case ValueRep::StackArgument:
- arg = Arg::callArg(value.rep().offsetFromSP());
- appendStore(value.value(), arg);
- break;
- default:
- RELEASE_ASSERT_NOT_REACHED();
- break;
- }
- inst.args.append(arg);
- }
+ fillStackmap(inst, patchpointValue, 0);
insts.last().append(WTF::move(inst));
return true;
}
+ CheckSpecial* checkBranchTest32Special { nullptr };
+ CheckSpecial* checkBranchTest64Special { nullptr };
+ bool tryCheck(Value* value)
+ {
+ if (!isInt(value->type())) {
+ // FIXME: Implement double branches.
+ // https://bugs.webkit.org/show_bug.cgi?id=150727
+ return false;
+ }
+
+ CheckSpecial* special;
+ switch (value->type()) {
+ case Int32:
+ special = ensureSpecial(checkBranchTest32Special, BranchTest32, 3);
+ break;
+ case Int64:
+ special = ensureSpecial(checkBranchTest64Special, BranchTest64, 3);
+ break;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ break;
+ }
+
+ CheckValue* checkValue = currentValue->as<CheckValue>();
+
+ Inst inst(
+ Patch, checkValue, Arg::special(special),
+ Arg::resCond(MacroAssembler::NonZero), tmp(value), Arg::imm(-1));
+
+ fillStackmap(inst, checkValue, 1);
+
+ insts.last().append(WTF::move(inst));
+ return true;
+ }
+
bool tryUpsilon(Value* value)
{
append(
Modified: trunk/Source/_javascript_Core/b3/B3LoweringMatcher.patterns (192034 => 192035)
--- trunk/Source/_javascript_Core/b3/B3LoweringMatcher.patterns 2015-11-04 21:46:10 UTC (rev 192034)
+++ trunk/Source/_javascript_Core/b3/B3LoweringMatcher.patterns 2015-11-04 22:15:00 UTC (rev 192035)
@@ -56,6 +56,7 @@
FramePointer = FramePointer()
Patchpoint = Patchpoint()
+Check = Check(value)
Upsilon = Upsilon(value)
Phi = Phi()
Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (192034 => 192035)
--- trunk/Source/_javascript_Core/b3/testb3.cpp 2015-11-04 21:46:10 UTC (rev 192034)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp 2015-11-04 22:15:00 UTC (rev 192035)
@@ -1996,6 +1996,33 @@
CHECK(compileAndRun<int>(proc, 1, 2) == 3);
}
+void testSimpleCheck()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+ Value* arg = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+ CheckValue* check = root->appendNew<CheckValue>(proc, Check, Origin(), arg);
+ check->setGenerator(
+ [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+ CHECK(params.reps.size() == 1);
+ CHECK(params.reps[0].isConstant());
+ CHECK(params.reps[0].value() == 1);
+
+ // This should always work because a function this simple should never have callee
+ // saves.
+ jit.move(CCallHelpers::TrustedImm32(42), GPRInfo::returnValueGPR);
+ jit.emitFunctionEpilogue();
+ jit.ret();
+ });
+ root->appendNew<ControlValue>(
+ proc, Return, Origin(), root->appendNew<Const32Value>(proc, Origin(), 0));
+
+ MacroAssemblerCodeRef code = compile(proc);
+
+ CHECK(invoke<int>(code, 0) == 0);
+ CHECK(invoke<int>(code, 1) == 42);
+}
+
#define RUN(test) do { \
if (!shouldRun(#test)) \
break; \
@@ -2271,6 +2298,7 @@
RUN(testComplex(4, 384));
RUN(testSimplePatchpoint());
+ RUN(testSimpleCheck());
if (!didRun)
usage();
