Log Message
B3 should support trapping memory accesses https://bugs.webkit.org/show_bug.cgi?id=162689
Reviewed by Geoffrey Garen.
This adds a traps flag to B3::Kind. It also makes B3::Kind work more like Air::Kind, in the
sense that it's a bag of distinct bits - it doesn't need to be a union unless we get enough
things that it would make a difference.
The only analysis that needs to know about traps is effects. It now knows that traps implies
sideExits, which means that this turns off DCE. The only optimization that needs to know
about traps is eliminateCommonSubexpressions(), which needs to pessimize its store
elimination if the store traps.
The hard part of this change is teaching the instruction selector to faithfully carry the
traps flag down to Air. I got this to work by making ArgPromise a non-copyable object that
knows whether you've used it in an instruction. It knows when you call consume(). If you do
this then ArgPromise cannot be destructed without first passing your inst through it. This,
along with a few other hacks, means that all of the load-op and load-op-store fusions
correctly carry the trap bit: if any of the B3 loads or stores involved traps then you get
traps in Air.
This framework also sets us up to do bug 162688, since the ArgPromise::inst() hook is
powerful enough to allow wrapping the instruction with a Patch.
I added some tests to testb3 that verify that optimizations are appropriately inhibited and
that the traps flag survives until the bitter end of Air.
* b3/B3EliminateCommonSubexpressions.cpp:
* b3/B3Kind.cpp:
(JSC::B3::Kind::dump):
* b3/B3Kind.h:
(JSC::B3::Kind::Kind):
(JSC::B3::Kind::hasExtraBits):
(JSC::B3::Kind::isChill):
(JSC::B3::Kind::setIsChill):
(JSC::B3::Kind::hasTraps):
(JSC::B3::Kind::traps):
(JSC::B3::Kind::setTraps):
(JSC::B3::Kind::operator==):
(JSC::B3::Kind::hash):
(JSC::B3::trapping):
* b3/B3LowerToAir.cpp:
(JSC::B3::Air::LowerToAir::ArgPromise::swap):
(JSC::B3::Air::LowerToAir::ArgPromise::ArgPromise):
(JSC::B3::Air::LowerToAir::ArgPromise::operator=):
(JSC::B3::Air::LowerToAir::ArgPromise::~ArgPromise):
(JSC::B3::Air::LowerToAir::ArgPromise::setTraps):
(JSC::B3::Air::LowerToAir::ArgPromise::consume):
(JSC::B3::Air::LowerToAir::ArgPromise::inst):
(JSC::B3::Air::LowerToAir::trappingInst):
(JSC::B3::Air::LowerToAir::loadPromiseAnyOpcode):
(JSC::B3::Air::LowerToAir::appendUnOp):
(JSC::B3::Air::LowerToAir::appendBinOp):
(JSC::B3::Air::LowerToAir::tryAppendStoreUnOp):
(JSC::B3::Air::LowerToAir::tryAppendStoreBinOp):
(JSC::B3::Air::LowerToAir::appendStore):
(JSC::B3::Air::LowerToAir::append):
(JSC::B3::Air::LowerToAir::createGenericCompare):
(JSC::B3::Air::LowerToAir::createBranch):
(JSC::B3::Air::LowerToAir::createCompare):
(JSC::B3::Air::LowerToAir::createSelect):
(JSC::B3::Air::LowerToAir::lower):
* b3/B3Validate.cpp:
* b3/B3Value.cpp:
(JSC::B3::Value::effects):
* b3/B3Value.h:
* b3/air/AirCode.h:
* b3/testb3.cpp:
(JSC::B3::testTrappingLoad):
(JSC::B3::testTrappingStore):
(JSC::B3::testTrappingLoadAddStore):
(JSC::B3::testTrappingLoadDCE):
(JSC::B3::testTrappingStoreElimination):
(JSC::B3::run):
Modified Paths
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp
- trunk/Source/_javascript_Core/b3/B3Kind.cpp
- trunk/Source/_javascript_Core/b3/B3Kind.h
- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp
- trunk/Source/_javascript_Core/b3/B3Validate.cpp
- trunk/Source/_javascript_Core/b3/B3Value.cpp
- trunk/Source/_javascript_Core/b3/B3Value.h
- trunk/Source/_javascript_Core/b3/air/AirCode.h
- trunk/Source/_javascript_Core/b3/testb3.cpp
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (206693 => 206694)
--- trunk/Source/_javascript_Core/ChangeLog 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-10-01 00:08:28 UTC (rev 206694)
@@ -1,3 +1,81 @@
+2016-09-30 Filip Pizlo <[email protected]>
+
+ B3 should support trapping memory accesses
+ https://bugs.webkit.org/show_bug.cgi?id=162689
+
+ Reviewed by Geoffrey Garen.
+
+ This adds a traps flag to B3::Kind. It also makes B3::Kind work more like Air::Kind, in the
+ sense that it's a bag of distinct bits - it doesn't need to be a union unless we get enough
+ things that it would make a difference.
+
+ The only analysis that needs to know about traps is effects. It now knows that traps implies
+ sideExits, which means that this turns off DCE. The only optimization that needs to know
+ about traps is eliminateCommonSubexpressions(), which needs to pessimize its store
+ elimination if the store traps.
+
+ The hard part of this change is teaching the instruction selector to faithfully carry the
+ traps flag down to Air. I got this to work by making ArgPromise a non-copyable object that
+ knows whether you've used it in an instruction. It knows when you call consume(). If you do
+ this then ArgPromise cannot be destructed without first passing your inst through it. This,
+ along with a few other hacks, means that all of the load-op and load-op-store fusions
+ correctly carry the trap bit: if any of the B3 loads or stores involved traps then you get
+ traps in Air.
+
+ This framework also sets us up to do bug 162688, since the ArgPromise::inst() hook is
+ powerful enough to allow wrapping the instruction with a Patch.
+
+ I added some tests to testb3 that verify that optimizations are appropriately inhibited and
+ that the traps flag survives until the bitter end of Air.
+
+ * b3/B3EliminateCommonSubexpressions.cpp:
+ * b3/B3Kind.cpp:
+ (JSC::B3::Kind::dump):
+ * b3/B3Kind.h:
+ (JSC::B3::Kind::Kind):
+ (JSC::B3::Kind::hasExtraBits):
+ (JSC::B3::Kind::isChill):
+ (JSC::B3::Kind::setIsChill):
+ (JSC::B3::Kind::hasTraps):
+ (JSC::B3::Kind::traps):
+ (JSC::B3::Kind::setTraps):
+ (JSC::B3::Kind::operator==):
+ (JSC::B3::Kind::hash):
+ (JSC::B3::trapping):
+ * b3/B3LowerToAir.cpp:
+ (JSC::B3::Air::LowerToAir::ArgPromise::swap):
+ (JSC::B3::Air::LowerToAir::ArgPromise::ArgPromise):
+ (JSC::B3::Air::LowerToAir::ArgPromise::operator=):
+ (JSC::B3::Air::LowerToAir::ArgPromise::~ArgPromise):
+ (JSC::B3::Air::LowerToAir::ArgPromise::setTraps):
+ (JSC::B3::Air::LowerToAir::ArgPromise::consume):
+ (JSC::B3::Air::LowerToAir::ArgPromise::inst):
+ (JSC::B3::Air::LowerToAir::trappingInst):
+ (JSC::B3::Air::LowerToAir::loadPromiseAnyOpcode):
+ (JSC::B3::Air::LowerToAir::appendUnOp):
+ (JSC::B3::Air::LowerToAir::appendBinOp):
+ (JSC::B3::Air::LowerToAir::tryAppendStoreUnOp):
+ (JSC::B3::Air::LowerToAir::tryAppendStoreBinOp):
+ (JSC::B3::Air::LowerToAir::appendStore):
+ (JSC::B3::Air::LowerToAir::append):
+ (JSC::B3::Air::LowerToAir::createGenericCompare):
+ (JSC::B3::Air::LowerToAir::createBranch):
+ (JSC::B3::Air::LowerToAir::createCompare):
+ (JSC::B3::Air::LowerToAir::createSelect):
+ (JSC::B3::Air::LowerToAir::lower):
+ * b3/B3Validate.cpp:
+ * b3/B3Value.cpp:
+ (JSC::B3::Value::effects):
+ * b3/B3Value.h:
+ * b3/air/AirCode.h:
+ * b3/testb3.cpp:
+ (JSC::B3::testTrappingLoad):
+ (JSC::B3::testTrappingStore):
+ (JSC::B3::testTrappingLoadAddStore):
+ (JSC::B3::testTrappingLoadDCE):
+ (JSC::B3::testTrappingStoreElimination):
+ (JSC::B3::run):
+
2016-09-30 Joseph Pecoraro <[email protected]>
Web Inspector: Stepping over/out of a function sometimes resumes instead of taking you to caller
Modified: trunk/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp 2016-10-01 00:08:28 UTC (rev 206694)
@@ -466,7 +466,7 @@
template<typename Filter>
void handleStoreAfterClobber(Value* ptr, HeapRange range, const Filter& filter)
{
- if (findStoreAfterClobber(ptr, range, filter)) {
+ if (!m_value->traps() && findStoreAfterClobber(ptr, range, filter)) {
m_value->replaceWithNop();
m_changed = true;
return;
Modified: trunk/Source/_javascript_Core/b3/B3Kind.cpp (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/B3Kind.cpp 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/B3Kind.cpp 2016-10-01 00:08:28 UTC (rev 206694)
@@ -39,6 +39,8 @@
CommaPrinter comma(", ", "<");
if (isChill())
out.print(comma, "Chill");
+ if (traps())
+ out.print(comma, "Traps");
if (comma.didPrint())
out.print(">");
}
Modified: trunk/Source/_javascript_Core/b3/B3Kind.h (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/B3Kind.h 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/B3Kind.h 2016-10-01 00:08:28 UTC (rev 206694)
@@ -60,8 +60,9 @@
public:
Kind(Opcode opcode)
: m_opcode(opcode)
+ , m_isChill(false)
+ , m_traps(false)
{
- u.bits = 0;
}
Kind()
@@ -72,7 +73,7 @@
Opcode opcode() const { return m_opcode; }
void setOpcode(Opcode opcode) { m_opcode = opcode; }
- bool hasExtraBits() const { return !!u.bits; }
+ bool hasExtraBits() const { return m_isChill || m_traps; }
// Chill bit. This applies to division-based arithmetic ops, which may trap on some
// platforms or exhibit bizarre behavior when passed certain inputs. The non-chill
@@ -100,14 +101,46 @@
}
bool isChill() const
{
- return hasIsChill() && u.isChill;
+ return m_isChill;
}
void setIsChill(bool isChill)
{
ASSERT(hasIsChill());
- u.isChill = isChill;
+ m_isChill = isChill;
}
+ // Traps bit. This applies to memory access ops. It means that the instruction could
+ // trap as part of some check it performs, and that we mean to make this observable. This
+ // currently only applies to memory accesses (loads and stores). You don't get to find out where
+ // in the Procedure the trap happened. If you try to work it out using Origin, you'll have a bad
+ // time because the instruction selector is too sloppy with Origin().
+ // FIXME: https://bugs.webkit.org/show_bug.cgi?id=162688
+ bool hasTraps() const
+ {
+ switch (m_opcode) {
+ case Load8Z:
+ case Load8S:
+ case Load16Z:
+ case Load16S:
+ case Load:
+ case Store8:
+ case Store16:
+ case Store:
+ return true;
+ default:
+ return false;
+ }
+ }
+ bool traps() const
+ {
+ return m_traps;
+ }
+ void setTraps(bool traps)
+ {
+ ASSERT(hasTraps());
+ m_traps = traps;
+ }
+
// Rules for adding new properties:
// - Put the accessors here.
// - hasBlah() should check if the opcode allows for your property.
@@ -119,7 +152,8 @@
bool operator==(const Kind& other) const
{
return m_opcode == other.m_opcode
- && u.bits == other.u.bits;
+ && m_isChill == other.m_isChill
+ && m_traps == other.m_traps;
}
bool operator!=(const Kind& other) const
@@ -133,13 +167,14 @@
{
// It's almost certainly more important that this hash function is cheap to compute than
// anything else. We can live with some kind hash collisions.
- return m_opcode + u.bits * 111;
+ return m_opcode + (static_cast<unsigned>(m_isChill) << 16) + (static_cast<unsigned>(m_traps) << 7);
}
Kind(WTF::HashTableDeletedValueType)
: m_opcode(Oops)
+ , m_isChill(true)
+ , m_traps(false)
{
- u.bits = 1;
}
bool isHashTableDeletedValue() const
@@ -149,10 +184,8 @@
private:
Opcode m_opcode;
- union {
- bool isChill;
- uint16_t bits;
- } u;
+ bool m_isChill : 1;
+ bool m_traps : 1;
};
// For every flag 'foo' you add, it's customary to create a Kind B3::foo(Kind) function that makes
@@ -160,7 +193,8 @@
//
// block->appendNew<Value>(m_proc, chill(Mod), Origin(), a, b);
//
-// That looks pretty slick. Let's keep it that way.
+// I like to make the flag name fill in the sentence "Mod _____" (like "isChill" or "traps") while
+// the flag constructor fills in the phrase "_____ Mod" (like "chill" or "trapping").
inline Kind chill(Kind kind)
{
@@ -168,6 +202,12 @@
return kind;
}
+inline Kind trapping(Kind kind)
+{
+ kind.setTraps(true);
+ return kind;
+}
+
struct KindHash {
static unsigned hash(const Kind& key) { return key.hash(); }
static bool equal(const Kind& a, const Kind& b) { return a == b; }
Modified: trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2016-10-01 00:08:28 UTC (rev 206694)
@@ -181,6 +181,7 @@
}
class ArgPromise {
+ WTF_MAKE_NONCOPYABLE(ArgPromise);
public:
ArgPromise() { }
@@ -189,6 +190,37 @@
, m_value(valueToLock)
{
}
+
+ void swap(ArgPromise& other)
+ {
+ std::swap(m_arg, other.m_arg);
+ std::swap(m_value, other.m_value);
+ std::swap(m_wasConsumed, other.m_wasConsumed);
+ std::swap(m_wasWrapped, other.m_wasWrapped);
+ std::swap(m_traps, other.m_traps);
+ }
+
+ ArgPromise(ArgPromise&& other)
+ {
+ swap(other);
+ }
+
+ ArgPromise& operator=(ArgPromise&& other)
+ {
+ swap(other);
+ return *this;
+ }
+
+ ~ArgPromise()
+ {
+ if (m_wasConsumed)
+ RELEASE_ASSERT(m_wasWrapped);
+ }
+
+ void setTraps(bool value)
+ {
+ m_traps = value;
+ }
static ArgPromise tmp(Value* value)
{
@@ -211,8 +243,9 @@
return m_arg;
}
- Arg consume(LowerToAir& lower) const
+ Arg consume(LowerToAir& lower)
{
+ m_wasConsumed = true;
if (!m_arg && m_value)
return lower.tmp(m_value);
if (m_value)
@@ -220,6 +253,15 @@
return m_arg;
}
+ template<typename... Args>
+ Inst inst(Args&&... args)
+ {
+ Inst result(std::forward<Args>(args)...);
+ result.kind.traps |= m_traps;
+ m_wasWrapped = true;
+ return result;
+ }
+
private:
// Three forms:
// Everything null: invalid.
@@ -227,7 +269,10 @@
// Arg null, value non-null: it's a tmp, pin it when necessary.
// Arg non-null, value non-null: use the arg, lock the value.
Arg m_arg;
- Value* m_value;
+ Value* m_value { nullptr };
+ bool m_wasConsumed { false };
+ bool m_wasWrapped { false };
+ bool m_traps { false };
};
// Consider using tmpPromise() in cases where you aren't sure that you want to pin the value yet.
@@ -246,11 +291,12 @@
// append(Foo, tmp(bar))
//
// Idiom #3: Same as Idiom #2, but using tmpPromise. Notice that this calls consume() only after
- // it's sure it will use the tmp. That's deliberate.
+ // it's sure it will use the tmp. That's deliberate. Also note that you're required to pass any
+ // Inst you create with consumed promises through that promise's inst() function.
//
// ArgPromise promise = tmpPromise(bar);
// if (isValidForm(Foo, promise.kind()))
- // append(Foo, promise.consume(*this))
+ // append(promise.inst(Foo, promise.consume(*this)))
//
// In both idiom #2 and idiom #3, we don't pin the value to a temporary except when we actually
// emit the instruction. Both tmp() and tmpPromise().consume(*this) will pin it. Pinning means
@@ -288,9 +334,9 @@
// loadAddr() helper and require you to balance ArgPromise's in code like this. Such code will
// work fine if written as:
//
- // auto tryThings = [this] (const ArgPromise& left, const ArgPromise& right) {
+ // auto tryThings = [this] (ArgPromise& left, ArgPromise& right) {
// if (isValidForm(Foo, left.kind(), right.kind()))
- // return Inst(Foo, m_value, left.consume(*this), right.consume(*this));
+ // return left.inst(right.inst(Foo, m_value, left.consume(*this), right.consume(*this)));
// return Inst();
// };
// if (Inst result = tryThings(loadPromise(left), tmpPromise(right)))
@@ -466,7 +512,21 @@
return result;
}
-
+
+ template<typename... Args>
+ Inst trappingInst(bool traps, Args&&... args)
+ {
+ Inst result(std::forward<Args>(args)...);
+ result.kind.traps |= traps;
+ return result;
+ }
+
+ template<typename... Args>
+ Inst trappingInst(Value* value, Args&&... args)
+ {
+ return trappingInst(value->traps(), std::forward<Args>(args)...);
+ }
+
ArgPromise loadPromiseAnyOpcode(Value* loadValue)
{
if (!canBeInternal(loadValue))
@@ -473,7 +533,10 @@
return Arg();
if (crossesInterference(loadValue))
return Arg();
- return ArgPromise(addr(loadValue), loadValue);
+ ArgPromise result(addr(loadValue), loadValue);
+ if (loadValue->traps())
+ result.setTraps(true);
+ return result;
}
ArgPromise loadPromise(Value* loadValue, B3::Opcode loadOpcode)
@@ -583,7 +646,7 @@
ArgPromise addr = loadPromise(value);
if (isValidForm(opcode, addr.kind(), Arg::Tmp)) {
- append(opcode, addr.consume(*this), result);
+ append(addr.inst(opcode, m_value, addr.consume(*this), result));
return;
}
@@ -717,7 +780,7 @@
if (left != right) {
ArgPromise leftAddr = loadPromise(left);
if (isValidForm(opcode, leftAddr.kind(), Arg::Tmp, Arg::Tmp)) {
- append(opcode, leftAddr.consume(*this), tmp(right), result);
+ append(leftAddr.inst(opcode, m_value, leftAddr.consume(*this), tmp(right), result));
return;
}
@@ -724,7 +787,7 @@
if (commutativity == Commutative) {
if (isValidForm(opcode, leftAddr.kind(), Arg::Tmp)) {
append(relaxedMoveForType(m_value->type()), tmp(right), result);
- append(opcode, leftAddr.consume(*this), result);
+ append(leftAddr.inst(opcode, m_value, leftAddr.consume(*this), result));
return;
}
}
@@ -731,13 +794,13 @@
ArgPromise rightAddr = loadPromise(right);
if (isValidForm(opcode, Arg::Tmp, rightAddr.kind(), Arg::Tmp)) {
- append(opcode, tmp(left), rightAddr.consume(*this), result);
+ append(rightAddr.inst(opcode, m_value, tmp(left), rightAddr.consume(*this), result));
return;
}
if (commutativity == Commutative) {
if (isValidForm(opcode, rightAddr.kind(), Arg::Tmp, Arg::Tmp)) {
- append(opcode, rightAddr.consume(*this), tmp(left), result);
+ append(rightAddr.inst(opcode, m_value, rightAddr.consume(*this), tmp(left), result));
return;
}
}
@@ -744,7 +807,7 @@
if (isValidForm(opcode, rightAddr.kind(), Arg::Tmp)) {
append(relaxedMoveForType(m_value->type()), tmp(left), result);
- append(opcode, rightAddr.consume(*this), result);
+ append(rightAddr.inst(opcode, m_value, rightAddr.consume(*this), result));
return;
}
}
@@ -823,7 +886,7 @@
return false;
loadPromise.consume(*this);
- append(opcode, storeAddr);
+ append(trappingInst(m_value, loadPromise.inst(opcode, m_value, storeAddr)));
return true;
}
@@ -875,7 +938,7 @@
if (isValidForm(opcode, Arg::Imm, storeAddr.kind()) && imm(otherValue)) {
loadPromise.consume(*this);
- append(opcode, imm(otherValue), storeAddr);
+ append(trappingInst(m_value, loadPromise.inst(opcode, m_value, imm(otherValue), storeAddr)));
return true;
}
@@ -883,7 +946,7 @@
return false;
loadPromise.consume(*this);
- append(opcode, tmp(otherValue), storeAddr);
+ append(trappingInst(m_value, loadPromise.inst(opcode, m_value, tmp(otherValue), storeAddr)));
return true;
}
@@ -901,9 +964,10 @@
return createStore(moveOpcode, value, dest);
}
- void appendStore(Value* value, const Arg& dest)
+ template<typename... Args>
+ void appendStore(Args&&... args)
{
- m_insts.last().append(createStore(value, dest));
+ append(trappingInst(m_value, createStore(std::forward<Args>(args)...)));
}
Air::Opcode moveForType(Type type)
@@ -963,6 +1027,15 @@
{
m_insts.last().append(Inst(opcode, m_value, std::forward<Arguments>(arguments)...));
}
+
+ void append(Inst&& inst)
+ {
+ m_insts.last().append(WTFMove(inst));
+ }
+ void append(const Inst& inst)
+ {
+ m_insts.last().append(inst);
+ }
template<typename T, typename... Arguments>
T* ensureSpecial(T*& field, Arguments&&... arguments)
@@ -1190,7 +1263,7 @@
Arg rightImm = imm(right);
auto tryCompare = [&] (
- Arg::Width width, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ Arg::Width width, ArgPromise&& left, ArgPromise&& right) -> Inst {
if (Inst result = compare(width, relCond, left, right))
return result;
if (Inst result = compare(width, relCond.flipped(), right, left))
@@ -1279,13 +1352,17 @@
}
// Finally, handle comparison between tmps.
- return compare(width, relCond, tmpPromise(left), tmpPromise(right));
+ ArgPromise leftPromise = tmpPromise(left);
+ ArgPromise rightPromise = tmpPromise(right);
+ return compare(width, relCond, leftPromise, rightPromise);
}
// Floating point comparisons can't really do anything smart.
+ ArgPromise leftPromise = tmpPromise(left);
+ ArgPromise rightPromise = tmpPromise(right);
if (value->child(0)->type() == Float)
- return compareFloat(doubleCond, tmpPromise(left), tmpPromise(right));
- return compareDouble(doubleCond, tmpPromise(left), tmpPromise(right));
+ return compareFloat(doubleCond, leftPromise, rightPromise);
+ return compareDouble(doubleCond, leftPromise, rightPromise);
};
Arg::Width width = Arg::widthForB3Type(value->type());
@@ -1292,7 +1369,7 @@
Arg resCond = Arg::resCond(MacroAssembler::NonZero).inverted(inverted);
auto tryTest = [&] (
- Arg::Width width, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ Arg::Width width, ArgPromise&& left, ArgPromise&& right) -> Inst {
if (Inst result = test(width, resCond, left, right))
return result;
if (Inst result = test(width, resCond, right, left))
@@ -1420,8 +1497,7 @@
if (hasRightConst) {
if ((width == Arg::Width32 && rightConst == 0xffffffff)
|| (width == Arg::Width64 && rightConst == -1)) {
- ArgPromise argPromise = tmpPromise(left);
- if (Inst result = tryTest(width, argPromise, argPromise))
+ if (Inst result = tryTest(width, tmpPromise(left), tmpPromise(left)))
return result;
}
if (isRepresentableAs<uint32_t>(rightConst)) {
@@ -1481,13 +1557,17 @@
}
}
- if (Inst result = test(width, resCond, tmpPromise(value), Arg::bitImm(-1)))
+ ArgPromise leftPromise = tmpPromise(value);
+ ArgPromise rightPromise = Arg::bitImm(-1);
+ if (Inst result = test(width, resCond, leftPromise, rightPromise))
return result;
}
// Sometimes this is the only form of test available. We prefer not to use this because
// it's less canonical.
- return test(width, resCond, tmpPromise(value), tmpPromise(value));
+ ArgPromise leftPromise = tmpPromise(value);
+ ArgPromise rightPromise = tmpPromise(value);
+ return test(width, resCond, leftPromise, rightPromise);
}
Inst createBranch(Value* value, bool inverted = false)
@@ -1496,13 +1576,13 @@
value,
[this] (
Arg::Width width, const Arg& relCond,
- const ArgPromise& left, const ArgPromise& right) -> Inst {
+ ArgPromise& left, ArgPromise& right) -> Inst {
switch (width) {
case Arg::Width8:
if (isValidForm(Branch8, Arg::RelCond, left.kind(), right.kind())) {
- return Inst(
+ return left.inst(right.inst(
Branch8, m_value, relCond,
- left.consume(*this), right.consume(*this));
+ left.consume(*this), right.consume(*this)));
}
return Inst();
case Arg::Width16:
@@ -1509,16 +1589,16 @@
return Inst();
case Arg::Width32:
if (isValidForm(Branch32, Arg::RelCond, left.kind(), right.kind())) {
- return Inst(
+ return left.inst(right.inst(
Branch32, m_value, relCond,
- left.consume(*this), right.consume(*this));
+ left.consume(*this), right.consume(*this)));
}
return Inst();
case Arg::Width64:
if (isValidForm(Branch64, Arg::RelCond, left.kind(), right.kind())) {
- return Inst(
+ return left.inst(right.inst(
Branch64, m_value, relCond,
- left.consume(*this), right.consume(*this));
+ left.consume(*this), right.consume(*this)));
}
return Inst();
}
@@ -1526,13 +1606,13 @@
},
[this] (
Arg::Width width, const Arg& resCond,
- const ArgPromise& left, const ArgPromise& right) -> Inst {
+ ArgPromise& left, ArgPromise& right) -> Inst {
switch (width) {
case Arg::Width8:
if (isValidForm(BranchTest8, Arg::ResCond, left.kind(), right.kind())) {
- return Inst(
+ return left.inst(right.inst(
BranchTest8, m_value, resCond,
- left.consume(*this), right.consume(*this));
+ left.consume(*this), right.consume(*this)));
}
return Inst();
case Arg::Width16:
@@ -1539,34 +1619,34 @@
return Inst();
case Arg::Width32:
if (isValidForm(BranchTest32, Arg::ResCond, left.kind(), right.kind())) {
- return Inst(
+ return left.inst(right.inst(
BranchTest32, m_value, resCond,
- left.consume(*this), right.consume(*this));
+ left.consume(*this), right.consume(*this)));
}
return Inst();
case Arg::Width64:
if (isValidForm(BranchTest64, Arg::ResCond, left.kind(), right.kind())) {
- return Inst(
+ return left.inst(right.inst(
BranchTest64, m_value, resCond,
- left.consume(*this), right.consume(*this));
+ left.consume(*this), right.consume(*this)));
}
return Inst();
}
ASSERT_NOT_REACHED();
},
- [this] (Arg doubleCond, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ [this] (Arg doubleCond, ArgPromise& left, ArgPromise& right) -> Inst {
if (isValidForm(BranchDouble, Arg::DoubleCond, left.kind(), right.kind())) {
- return Inst(
+ return left.inst(right.inst(
BranchDouble, m_value, doubleCond,
- left.consume(*this), right.consume(*this));
+ left.consume(*this), right.consume(*this)));
}
return Inst();
},
- [this] (Arg doubleCond, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ [this] (Arg doubleCond, ArgPromise& left, ArgPromise& right) -> Inst {
if (isValidForm(BranchFloat, Arg::DoubleCond, left.kind(), right.kind())) {
- return Inst(
+ return left.inst(right.inst(
BranchFloat, m_value, doubleCond,
- left.consume(*this), right.consume(*this));
+ left.consume(*this), right.consume(*this)));
}
return Inst();
},
@@ -1579,7 +1659,7 @@
value,
[this] (
Arg::Width width, const Arg& relCond,
- const ArgPromise& left, const ArgPromise& right) -> Inst {
+ ArgPromise& left, ArgPromise& right) -> Inst {
switch (width) {
case Arg::Width8:
case Arg::Width16:
@@ -1586,16 +1666,16 @@
return Inst();
case Arg::Width32:
if (isValidForm(Compare32, Arg::RelCond, left.kind(), right.kind(), Arg::Tmp)) {
- return Inst(
+ return left.inst(right.inst(
Compare32, m_value, relCond,
- left.consume(*this), right.consume(*this), tmp(m_value));
+ left.consume(*this), right.consume(*this), tmp(m_value)));
}
return Inst();
case Arg::Width64:
if (isValidForm(Compare64, Arg::RelCond, left.kind(), right.kind(), Arg::Tmp)) {
- return Inst(
+ return left.inst(right.inst(
Compare64, m_value, relCond,
- left.consume(*this), right.consume(*this), tmp(m_value));
+ left.consume(*this), right.consume(*this), tmp(m_value)));
}
return Inst();
}
@@ -1603,7 +1683,7 @@
},
[this] (
Arg::Width width, const Arg& resCond,
- const ArgPromise& left, const ArgPromise& right) -> Inst {
+ ArgPromise& left, ArgPromise& right) -> Inst {
switch (width) {
case Arg::Width8:
case Arg::Width16:
@@ -1610,34 +1690,34 @@
return Inst();
case Arg::Width32:
if (isValidForm(Test32, Arg::ResCond, left.kind(), right.kind(), Arg::Tmp)) {
- return Inst(
+ return left.inst(right.inst(
Test32, m_value, resCond,
- left.consume(*this), right.consume(*this), tmp(m_value));
+ left.consume(*this), right.consume(*this), tmp(m_value)));
}
return Inst();
case Arg::Width64:
if (isValidForm(Test64, Arg::ResCond, left.kind(), right.kind(), Arg::Tmp)) {
- return Inst(
+ return left.inst(right.inst(
Test64, m_value, resCond,
- left.consume(*this), right.consume(*this), tmp(m_value));
+ left.consume(*this), right.consume(*this), tmp(m_value)));
}
return Inst();
}
ASSERT_NOT_REACHED();
},
- [this] (const Arg& doubleCond, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ [this] (const Arg& doubleCond, ArgPromise& left, ArgPromise& right) -> Inst {
if (isValidForm(CompareDouble, Arg::DoubleCond, left.kind(), right.kind(), Arg::Tmp)) {
- return Inst(
+ return left.inst(right.inst(
CompareDouble, m_value, doubleCond,
- left.consume(*this), right.consume(*this), tmp(m_value));
+ left.consume(*this), right.consume(*this), tmp(m_value)));
}
return Inst();
},
- [this] (const Arg& doubleCond, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ [this] (const Arg& doubleCond, ArgPromise& left, ArgPromise& right) -> Inst {
if (isValidForm(CompareFloat, Arg::DoubleCond, left.kind(), right.kind(), Arg::Tmp)) {
- return Inst(
+ return left.inst(right.inst(
CompareFloat, m_value, doubleCond,
- left.consume(*this), right.consume(*this), tmp(m_value));
+ left.consume(*this), right.consume(*this), tmp(m_value)));
}
return Inst();
},
@@ -1654,22 +1734,22 @@
};
Inst createSelect(const MoveConditionallyConfig& config)
{
- auto createSelectInstruction = [&] (Air::Opcode opcode, const Arg& condition, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ auto createSelectInstruction = [&] (Air::Opcode opcode, const Arg& condition, ArgPromise& left, ArgPromise& right) -> Inst {
if (isValidForm(opcode, condition.kind(), left.kind(), right.kind(), Arg::Tmp, Arg::Tmp, Arg::Tmp)) {
Tmp result = tmp(m_value);
Tmp thenCase = tmp(m_value->child(1));
Tmp elseCase = tmp(m_value->child(2));
- return Inst(
+ return left.inst(right.inst(
opcode, m_value, condition,
- left.consume(*this), right.consume(*this), thenCase, elseCase, result);
+ left.consume(*this), right.consume(*this), thenCase, elseCase, result));
}
if (isValidForm(opcode, condition.kind(), left.kind(), right.kind(), Arg::Tmp, Arg::Tmp)) {
Tmp result = tmp(m_value);
Tmp source = tmp(m_value->child(1));
append(relaxedMoveForType(m_value->type()), tmp(m_value->child(2)), result);
- return Inst(
+ return left.inst(right.inst(
opcode, m_value, condition,
- left.consume(*this), right.consume(*this), source, result);
+ left.consume(*this), right.consume(*this), source, result));
}
return Inst();
};
@@ -1678,7 +1758,7 @@
m_value->child(0),
[&] (
Arg::Width width, const Arg& relCond,
- const ArgPromise& left, const ArgPromise& right) -> Inst {
+ ArgPromise& left, ArgPromise& right) -> Inst {
switch (width) {
case Arg::Width8:
// FIXME: Support these things.
@@ -1695,7 +1775,7 @@
},
[&] (
Arg::Width width, const Arg& resCond,
- const ArgPromise& left, const ArgPromise& right) -> Inst {
+ ArgPromise& left, ArgPromise& right) -> Inst {
switch (width) {
case Arg::Width8:
// FIXME: Support more things.
@@ -1710,10 +1790,10 @@
}
ASSERT_NOT_REACHED();
},
- [&] (Arg doubleCond, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ [&] (Arg doubleCond, ArgPromise& left, ArgPromise& right) -> Inst {
return createSelectInstruction(config.moveConditionallyDouble, doubleCond, left, right);
},
- [&] (Arg doubleCond, const ArgPromise& left, const ArgPromise& right) -> Inst {
+ [&] (Arg doubleCond, ArgPromise& left, ArgPromise& right) -> Inst {
return createSelectInstruction(config.moveConditionallyFloat, doubleCond, left, right);
},
false);
@@ -1729,29 +1809,27 @@
}
case Load: {
- append(
- moveForType(m_value->type()),
- addr(m_value), tmp(m_value));
+ append(trappingInst(m_value, moveForType(m_value->type()), m_value, addr(m_value), tmp(m_value)));
return;
}
case Load8S: {
- append(Load8SignedExtendTo32, addr(m_value), tmp(m_value));
+ append(trappingInst(m_value, Load8SignedExtendTo32, m_value, addr(m_value), tmp(m_value)));
return;
}
case Load8Z: {
- append(Load8, addr(m_value), tmp(m_value));
+ append(trappingInst(m_value, Load8, m_value, addr(m_value), tmp(m_value)));
return;
}
case Load16S: {
- append(Load16SignedExtendTo32, addr(m_value), tmp(m_value));
+ append(trappingInst(m_value, Load16SignedExtendTo32, m_value, addr(m_value), tmp(m_value)));
return;
}
case Load16Z: {
- append(Load16, addr(m_value), tmp(m_value));
+ append(trappingInst(m_value, Load16, m_value, addr(m_value), tmp(m_value)));
return;
}
@@ -2022,7 +2100,7 @@
return;
}
}
- m_insts.last().append(createStore(Air::Store8, valueToStore, addr(m_value)));
+ appendStore(Air::Store8, valueToStore, addr(m_value));
return;
}
@@ -2043,7 +2121,7 @@
return;
}
}
- m_insts.last().append(createStore(Air::Store16, valueToStore, addr(m_value)));
+ appendStore(Air::Store16, valueToStore, addr(m_value));
return;
}
Modified: trunk/Source/_javascript_Core/b3/B3Validate.cpp (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/B3Validate.cpp 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/B3Validate.cpp 2016-10-01 00:08:28 UTC (rev 206694)
@@ -191,6 +191,7 @@
case Mod:
case BitAnd:
case BitXor:
+ VALIDATE(!value->kind().traps(), ("At ", *value));
switch (value->opcode()) {
case Div:
case Mod:
@@ -339,7 +340,7 @@
case Load8S:
case Load16Z:
case Load16S:
- VALIDATE(!value->kind().hasExtraBits(), ("At ", *value));
+ VALIDATE(!value->kind().isChill(), ("At ", *value));
VALIDATE(value->numChildren() == 1, ("At ", *value));
VALIDATE(value->child(0)->type() == pointerType(), ("At ", *value));
VALIDATE(value->type() == Int32, ("At ", *value));
@@ -346,7 +347,7 @@
validateStackAccess(value);
break;
case Load:
- VALIDATE(!value->kind().hasExtraBits(), ("At ", *value));
+ VALIDATE(!value->kind().isChill(), ("At ", *value));
VALIDATE(value->numChildren() == 1, ("At ", *value));
VALIDATE(value->child(0)->type() == pointerType(), ("At ", *value));
VALIDATE(value->type() != Void, ("At ", *value));
@@ -354,7 +355,7 @@
break;
case Store8:
case Store16:
- VALIDATE(!value->kind().hasExtraBits(), ("At ", *value));
+ VALIDATE(!value->kind().isChill(), ("At ", *value));
VALIDATE(value->numChildren() == 2, ("At ", *value));
VALIDATE(value->child(0)->type() == Int32, ("At ", *value));
VALIDATE(value->child(1)->type() == pointerType(), ("At ", *value));
@@ -362,7 +363,7 @@
validateStackAccess(value);
break;
case Store:
- VALIDATE(!value->kind().hasExtraBits(), ("At ", *value));
+ VALIDATE(!value->kind().isChill(), ("At ", *value));
VALIDATE(value->numChildren() == 2, ("At ", *value));
VALIDATE(value->child(1)->type() == pointerType(), ("At ", *value));
VALIDATE(value->type() == Void, ("At ", *value));
Modified: trunk/Source/_javascript_Core/b3/B3Value.cpp (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/B3Value.cpp 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/B3Value.cpp 2016-10-01 00:08:28 UTC (rev 206694)
@@ -635,6 +635,7 @@
result.terminal = true;
break;
}
+ result.exitsSideways |= traps();
return result;
}
Modified: trunk/Source/_javascript_Core/b3/B3Value.h (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/B3Value.h 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/B3Value.h 2016-10-01 00:08:28 UTC (rev 206694)
@@ -69,6 +69,7 @@
// It's good practice to mirror Kind methods here, so you can say value->isBlah()
// instead of value->kind().isBlah().
bool isChill() const { return kind().isChill(); }
+ bool traps() const { return kind().traps(); }
Origin origin() const { return m_origin; }
void setOrigin(Origin origin) { m_origin = origin; }
Modified: trunk/Source/_javascript_Core/b3/air/AirCode.h (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/air/AirCode.h 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/air/AirCode.h 2016-10-01 00:08:28 UTC (rev 206694)
@@ -162,7 +162,7 @@
Vector<std::unique_ptr<BasicBlock>>& blockList() { return m_blocks; }
// Finds the smallest index' such that at(index') != null and index' >= index.
- unsigned findFirstBlockIndex(unsigned index) const;
+ JS_EXPORT_PRIVATE unsigned findFirstBlockIndex(unsigned index) const;
// Finds the smallest index' such that at(index') != null and index' > index.
unsigned findNextBlockIndex(unsigned index) const;
Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (206693 => 206694)
--- trunk/Source/_javascript_Core/b3/testb3.cpp 2016-09-30 23:26:48 UTC (rev 206693)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp 2016-10-01 00:08:28 UTC (rev 206694)
@@ -13108,6 +13108,119 @@
checkDoesNotUseInstruction(*code, "dmb ishst");
}
+void testTrappingLoad()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+ int x = 42;
+ root->appendNew<Value>(
+ proc, Return, Origin(),
+ root->appendNew<MemoryValue>(
+ proc, trapping(Load), Int32, Origin(),
+ root->appendNew<ConstPtrValue>(proc, Origin(), &x)));
+ CHECK_EQ(compileAndRun<int>(proc), 42);
+ unsigned trapsCount = 0;
+ for (Air::BasicBlock* block : proc.code()) {
+ for (Air::Inst& inst : *block) {
+ if (inst.kind.traps)
+ trapsCount++;
+ }
+ }
+ CHECK_EQ(trapsCount, 1u);
+}
+
+void testTrappingStore()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+ int x = 42;
+ root->appendNew<MemoryValue>(
+ proc, trapping(Store), Origin(),
+ root->appendNew<Const32Value>(proc, Origin(), 111),
+ root->appendNew<ConstPtrValue>(proc, Origin(), &x));
+ root->appendNew<Value>(proc, Return, Origin());
+ compileAndRun<int>(proc);
+ CHECK_EQ(x, 111);
+ unsigned trapsCount = 0;
+ for (Air::BasicBlock* block : proc.code()) {
+ for (Air::Inst& inst : *block) {
+ if (inst.kind.traps)
+ trapsCount++;
+ }
+ }
+ CHECK_EQ(trapsCount, 1u);
+}
+
+void testTrappingLoadAddStore()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+ int x = 42;
+ ConstPtrValue* ptr = root->appendNew<ConstPtrValue>(proc, Origin(), &x);
+ root->appendNew<MemoryValue>(
+ proc, trapping(Store), Origin(),
+ root->appendNew<Value>(
+ proc, Add, Origin(),
+ root->appendNew<MemoryValue>(proc, trapping(Load), Int32, Origin(), ptr),
+ root->appendNew<Const32Value>(proc, Origin(), 3)),
+ ptr);
+ root->appendNew<Value>(proc, Return, Origin());
+ compileAndRun<int>(proc);
+ CHECK_EQ(x, 45);
+ bool traps = false;
+ for (Air::BasicBlock* block : proc.code()) {
+ for (Air::Inst& inst : *block) {
+ if (inst.kind.traps)
+ traps = true;
+ }
+ }
+ CHECK(traps);
+}
+
+void testTrappingLoadDCE()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+ int x = 42;
+ root->appendNew<MemoryValue>(
+ proc, trapping(Load), Int32, Origin(),
+ root->appendNew<ConstPtrValue>(proc, Origin(), &x));
+ root->appendNew<Value>(proc, Return, Origin());
+ compileAndRun<int>(proc);
+ unsigned trapsCount = 0;
+ for (Air::BasicBlock* block : proc.code()) {
+ for (Air::Inst& inst : *block) {
+ if (inst.kind.traps)
+ trapsCount++;
+ }
+ }
+ CHECK_EQ(trapsCount, 1u);
+}
+
+void testTrappingStoreElimination()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+ int x = 42;
+ Value* ptr = root->appendNew<ConstPtrValue>(proc, Origin(), &x);
+ root->appendNew<MemoryValue>(
+ proc, trapping(Store), Origin(),
+ root->appendNew<Const32Value>(proc, Origin(), 43),
+ ptr);
+ root->appendNew<MemoryValue>(
+ proc, trapping(Store), Origin(),
+ root->appendNew<Const32Value>(proc, Origin(), 44),
+ ptr);
+ root->appendNew<Value>(proc, Return, Origin());
+ compileAndRun<int>(proc);
+ unsigned storeCount = 0;
+ for (Value* value : proc.values()) {
+ if (MemoryValue::isStore(value->opcode()))
+ storeCount++;
+ }
+ CHECK_EQ(storeCount, 2u);
+}
+
void testMoveConstants()
{
auto check = [] (Procedure& proc) {
@@ -14609,6 +14722,11 @@
RUN(testMemoryFence());
RUN(testStoreFence());
RUN(testLoadFence());
+ RUN(testTrappingLoad());
+ RUN(testTrappingStore());
+ RUN(testTrappingLoadAddStore());
+ RUN(testTrappingLoadDCE());
+ RUN(testTrappingStoreElimination());
RUN(testMoveConstants());
if (tasks.isEmpty())
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
