Skip to content

Commit

Permalink
Bug 1590120 - Avoid repeated bailouts when MToString is used for jsop…
Browse files Browse the repository at this point in the history
…_tostring or the ToString intrinsic. r=anba

In these stand-alone cases we can handle objects/symbols by calling ToStringSlow.
The TypePolicy uses of MToString will still bail out to Baseline.

Differential Revision: https://phabricator.services.mozilla.com/D50797

--HG--
extra : moz-landing-system : lando
  • Loading branch information
jandem committed Oct 28, 2019
1 parent e50dce9 commit d84e72f
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 27 deletions.
30 changes: 19 additions & 11 deletions js/src/jit/CodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1718,20 +1718,28 @@ void CodeGenerator::visitValueToString(LValueToString* lir) {

// Object
if (lir->mir()->input()->mightBeType(MIRType::Object)) {
// Bail.
MOZ_ASSERT(lir->mir()->fallible());
Label bail;
masm.branchTestObject(Assembler::Equal, tag, &bail);
bailoutFrom(&bail, lir->snapshot());
if (lir->mir()->supportSideEffects()) {
masm.branchTestObject(Assembler::Equal, tag, ool->entry());
} else {
// Bail.
MOZ_ASSERT(lir->mir()->needsSnapshot());
Label bail;
masm.branchTestObject(Assembler::Equal, tag, &bail);
bailoutFrom(&bail, lir->snapshot());
}
}

// Symbol
if (lir->mir()->input()->mightBeType(MIRType::Symbol)) {
// Bail.
MOZ_ASSERT(lir->mir()->fallible());
Label bail;
masm.branchTestSymbol(Assembler::Equal, tag, &bail);
bailoutFrom(&bail, lir->snapshot());
if (lir->mir()->supportSideEffects()) {
masm.branchTestSymbol(Assembler::Equal, tag, ool->entry());
} else {
// Bail.
MOZ_ASSERT(lir->mir()->needsSnapshot());
Label bail;
masm.branchTestSymbol(Assembler::Equal, tag, &bail);
bailoutFrom(&bail, lir->snapshot());
}
}

// BigInt
Expand All @@ -1741,7 +1749,7 @@ void CodeGenerator::visitValueToString(LValueToString* lir) {
}

#ifdef DEBUG
masm.assumeUnreachable("Unexpected type for MValueToString.");
masm.assumeUnreachable("Unexpected type for LValueToString.");
#endif

masm.bind(&done);
Expand Down
7 changes: 5 additions & 2 deletions js/src/jit/IonBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4011,10 +4011,13 @@ AbortReasonOr<Ok> IonBuilder::jsop_tostring() {
}

MDefinition* value = current->pop();
MToString* ins = MToString::New(alloc(), value);
MToString* ins =
MToString::New(alloc(), value, MToString::SideEffectHandling::Supported);
current->add(ins);
current->push(ins);
MOZ_ASSERT(!ins->isEffectful());
if (ins->isEffectful()) {
MOZ_TRY(resumeAfter(ins));
}
return Ok();
}

Expand Down
2 changes: 1 addition & 1 deletion js/src/jit/Lowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2275,7 +2275,7 @@ void LIRGenerator::visitToString(MToString* ins) {
case MIRType::Value: {
LValueToString* lir =
new (alloc()) LValueToString(useBox(opd), tempToUnbox());
if (ins->fallible()) {
if (ins->needsSnapshot()) {
assignSnapshot(lir, Bailout_NonPrimitiveInput);
}
define(lir, ins);
Expand Down
6 changes: 5 additions & 1 deletion js/src/jit/MCallOptimize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3720,9 +3720,13 @@ IonBuilder::InliningResult IonBuilder::inlineToString(CallInfo& callInfo) {
}

callInfo.setImplicitlyUsedUnchecked();
MToString* toString = MToString::New(alloc(), callInfo.getArg(0));
MToString* toString = MToString::New(
alloc(), callInfo.getArg(0), MToString::SideEffectHandling::Supported);
current->add(toString);
current->push(toString);
if (toString->isEffectful()) {
MOZ_TRY(resumeAfter(toString));
}
return InliningStatus_Inlined;
}

Expand Down
57 changes: 46 additions & 11 deletions js/src/jit/MIR.h
Original file line number Diff line number Diff line change
Expand Up @@ -4207,33 +4207,68 @@ class MTruncateToInt32 : public MUnaryInstruction, public ToInt32Policy::Data {

// Converts any type to a string
class MToString : public MUnaryInstruction, public ToStringPolicy::Data {
explicit MToString(MDefinition* def) : MUnaryInstruction(classOpcode, def) {
public:
// MToString has two modes for handling of object/symbol arguments: if the
// to-string conversion happens as part of another opcode, we have to bail out
// to Baseline. If the conversion is for a stand-alone JSOp we can support
// side-effects.
enum class SideEffectHandling { Bailout, Supported };

private:
SideEffectHandling sideEffects_;

MToString(MDefinition* def, SideEffectHandling sideEffects)
: MUnaryInstruction(classOpcode, def), sideEffects_(sideEffects) {
setResultType(MIRType::String);
setMovable();

// Objects might override toString; Symbol throws. We bailout in those cases
// and run side-effects in baseline instead.
if (def->mightBeType(MIRType::Object) ||
def->mightBeType(MIRType::Symbol)) {
setGuard();
// If this instruction is not effectful, mark it as movable and set the
// Guard flag if needed. If the operation is effectful it won't be optimized
// anyway so there's no need to set any flags.
if (!isEffectful()) {
setMovable();
// Objects might override toString; Symbol throws. We bailout in those
// cases and run side-effects in baseline instead.
if (conversionMightHaveSideEffects()) {
setGuard();
}
}
}

bool conversionMightHaveSideEffects() const {
return input()->mightBeType(MIRType::Object) ||
input()->mightBeType(MIRType::Symbol);
}

public:
INSTRUCTION_HEADER(ToString)
TRIVIAL_NEW_WRAPPERS

MDefinition* foldsTo(TempAllocator& alloc) override;

bool congruentTo(const MDefinition* ins) const override {
if (!ins->isToString()) {
return false;
}
if (sideEffects_ != ins->toToString()->sideEffects_) {
return false;
}
return congruentIfOperandsEqual(ins);
}

AliasSet getAliasSet() const override { return AliasSet::None(); }
AliasSet getAliasSet() const override {
if (supportSideEffects() && conversionMightHaveSideEffects()) {
return AliasSet::Store(AliasSet::Any);
}
return AliasSet::None();
}

bool fallible() const {
return input()->mightBeType(MIRType::Object) ||
input()->mightBeType(MIRType::Symbol);
bool supportSideEffects() const {
return sideEffects_ == SideEffectHandling::Supported;
}

bool needsSnapshot() const {
return sideEffects_ == SideEffectHandling::Bailout &&
conversionMightHaveSideEffects();
}

ALLOW_CLONE(MToString)
Expand Down
3 changes: 2 additions & 1 deletion js/src/jit/TypePolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,8 @@ bool ConvertToStringPolicy<Op>::staticAdjustInputs(TempAllocator& alloc,
return true;
}

MToString* replace = MToString::New(alloc, in);
MToString* replace =
MToString::New(alloc, in, MToString::SideEffectHandling::Bailout);
ins->block()->insertBefore(ins, replace);
ins->replaceOperand(Op, replace);

Expand Down

0 comments on commit d84e72f

Please sign in to comment.