From 4b0b1f9e36a5f03976586c03bd9036129394dbbb Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 29 May 2015 13:37:59 +0200 Subject: [PATCH] Bug 1157624: Remove asm.js ternary optimizations and activate the FoldTest optimization pass for asm.js; r=luke --- js/src/asmjs/AsmJSValidate.cpp | 153 ++------------------------------- js/src/jit/Ion.cpp | 2 +- 2 files changed, 9 insertions(+), 146 deletions(-) diff --git a/js/src/asmjs/AsmJSValidate.cpp b/js/src/asmjs/AsmJSValidate.cpp index 8154a5af8f2..2596945cef8 100644 --- a/js/src/asmjs/AsmJSValidate.cpp +++ b/js/src/asmjs/AsmJSValidate.cpp @@ -6922,150 +6922,6 @@ CheckLabel(FunctionCompiler& f, ParseNode* labeledStmt, LabelVector* maybeLabels return f.bindLabeledBreaks(&labels, labeledStmt); } -static bool -CheckLeafCondition(FunctionCompiler& f, ParseNode* cond, ParseNode* thenStmt, ParseNode* elseOrJoinStmt, - MBasicBlock** thenBlock, MBasicBlock** elseOrJoinBlock) -{ - MDefinition* condDef; - Type condType; - if (!CheckExpr(f, cond, &condDef, &condType)) - return false; - if (!condType.isInt()) - return f.failf(cond, "%s is not a subtype of int", condType.toChars()); - - if (!f.branchAndStartThen(condDef, thenBlock, elseOrJoinBlock, thenStmt, elseOrJoinStmt)) - return false; - return true; -} - -static bool -CheckIfCondition(FunctionCompiler& f, ParseNode* cond, ParseNode* thenStmt, ParseNode* elseOrJoinStmt, - MBasicBlock** thenBlock, MBasicBlock** elseOrJoinBlock); - -static bool -CheckIfConditional(FunctionCompiler& f, ParseNode* conditional, ParseNode* thenStmt, ParseNode* elseOrJoinStmt, - MBasicBlock** thenBlock, MBasicBlock** elseOrJoinBlock) -{ - MOZ_ASSERT(conditional->isKind(PNK_CONDITIONAL)); - - // a ? b : c <=> (a && b) || (!a && c) - // b is always referred to the AND condition, as we need A and B to reach this test, - // c is always referred as the OR condition, as we reach it if we don't have A. - ParseNode* cond = TernaryKid1(conditional); - ParseNode* lhs = TernaryKid2(conditional); - ParseNode* rhs = TernaryKid3(conditional); - - MBasicBlock* maybeAndTest = nullptr; - MBasicBlock* maybeOrTest = nullptr; - MBasicBlock** ifTrueBlock = &maybeAndTest; - MBasicBlock** ifFalseBlock = &maybeOrTest; - ParseNode* ifTrueBlockNode = lhs; - ParseNode* ifFalseBlockNode = rhs; - - // Try to spot opportunities for short-circuiting in the AND subpart - uint32_t andTestLiteral = 0; - bool skipAndTest = false; - - if (IsLiteralInt(f.m(), lhs, &andTestLiteral)) { - skipAndTest = true; - if (andTestLiteral == 0) { - // (a ? 0 : b) is equivalent to !a && b - // If a is true, jump to the elseBlock directly - ifTrueBlock = elseOrJoinBlock; - ifTrueBlockNode = elseOrJoinStmt; - } else { - // (a ? 1 : b) is equivalent to a || b - // If a is true, jump to the thenBlock directly - ifTrueBlock = thenBlock; - ifTrueBlockNode = thenStmt; - } - } - - // Try to spot opportunities for short-circuiting in the OR subpart - uint32_t orTestLiteral = 0; - bool skipOrTest = false; - - if (IsLiteralInt(f.m(), rhs, &orTestLiteral)) { - skipOrTest = true; - if (orTestLiteral == 0) { - // (a ? b : 0) is equivalent to a && b - // If a is false, jump to the elseBlock directly - ifFalseBlock = elseOrJoinBlock; - ifFalseBlockNode = elseOrJoinStmt; - } else { - // (a ? b : 1) is equivalent to !a || b - // If a is false, jump to the thenBlock directly - ifFalseBlock = thenBlock; - ifFalseBlockNode = thenStmt; - } - } - - // Pathological cases: a ? 0 : 0 (i.e. false) or a ? 1 : 1 (i.e. true) - // These cases can't be optimized properly at this point: one of the blocks might be - // created and won't ever be executed. Furthermore, it introduces inconsistencies in the - // MIR graph (even if we try to create a block by hand, it will have no predecessor, which - // breaks graph assumptions). The only way we could optimize it is to do it directly in - // CheckIf by removing the control flow entirely. - if (skipOrTest && skipAndTest && (!!orTestLiteral == !!andTestLiteral)) - return CheckLeafCondition(f, conditional, thenStmt, elseOrJoinStmt, thenBlock, elseOrJoinBlock); - - if (!CheckIfCondition(f, cond, ifTrueBlockNode, ifFalseBlockNode, ifTrueBlock, ifFalseBlock)) - return false; - f.assertCurrentBlockIs(*ifTrueBlock); - - // Add supplementary tests, if needed - if (!skipAndTest) { - if (!CheckIfCondition(f, lhs, thenStmt, elseOrJoinStmt, thenBlock, elseOrJoinBlock)) - return false; - f.assertCurrentBlockIs(*thenBlock); - } - - if (!skipOrTest) { - f.switchToElse(*ifFalseBlock); - if (!CheckIfCondition(f, rhs, thenStmt, elseOrJoinStmt, thenBlock, elseOrJoinBlock)) - return false; - f.assertCurrentBlockIs(*thenBlock); - } - - // We might not be on the thenBlock in one case - if (ifTrueBlock == elseOrJoinBlock) { - MOZ_ASSERT(skipAndTest && andTestLiteral == 0); - f.switchToElse(*thenBlock); - } - - // Check post-conditions - f.assertCurrentBlockIs(*thenBlock); - MOZ_ASSERT_IF(!f.inDeadCode(), *thenBlock && *elseOrJoinBlock); - return true; -} - -// Recursive function that checks for a complex condition (formed with ternary -// conditionals) and creates the associated short-circuiting control flow graph. -// -// After a call to CheckCondition, the followings are true: -// - if *thenBlock and *elseOrJoinBlock were non-null on entry, their value is -// not changed by this function. -// - *thenBlock and *elseOrJoinBlock are non-null on exit. -// - the current block on exit is the *thenBlock. -static bool -CheckIfCondition(FunctionCompiler& f, ParseNode* cond, ParseNode* thenStmt, - ParseNode* elseOrJoinStmt, MBasicBlock** thenBlock, MBasicBlock** elseOrJoinBlock) -{ - JS_CHECK_RECURSION_DONT_REPORT(f.cx(), return f.m().failOverRecursed()); - - if (cond->isKind(PNK_CONDITIONAL)) - return CheckIfConditional(f, cond, thenStmt, elseOrJoinStmt, thenBlock, elseOrJoinBlock); - - // We've reached a leaf, i.e. an atomic condition - if (!CheckLeafCondition(f, cond, thenStmt, elseOrJoinStmt, thenBlock, elseOrJoinBlock)) - return false; - - // Check post-conditions - f.assertCurrentBlockIs(*thenBlock); - MOZ_ASSERT_IF(!f.inDeadCode(), *thenBlock && *elseOrJoinBlock); - return true; -} - static bool CheckIf(FunctionCompiler& f, ParseNode* ifStmt) { @@ -7086,7 +6942,14 @@ CheckIf(FunctionCompiler& f, ParseNode* ifStmt) MBasicBlock* elseBlock = nullptr; ParseNode* elseOrJoinStmt = elseStmt ? elseStmt : nextStmt; - if (!CheckIfCondition(f, cond, thenStmt, elseOrJoinStmt, &thenBlock, &elseBlock)) + MDefinition* condDef; + Type condType; + if (!CheckExpr(f, cond, &condDef, &condType)) + return false; + if (!condType.isInt()) + return f.failf(cond, "%s is not a subtype of int", condType.toChars()); + + if (!f.branchAndStartThen(condDef, &thenBlock, &elseBlock, thenStmt, elseOrJoinStmt)) return false; if (!CheckStatement(f, thenStmt)) diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp index 2c76b057f3b..66040e92d34 100644 --- a/js/src/jit/Ion.cpp +++ b/js/src/jit/Ion.cpp @@ -1279,7 +1279,7 @@ OptimizeMIR(MIRGenerator* mir) if (mir->shouldCancel("Start")) return false; - if (!mir->compilingAsmJS()) { + { AutoTraceLog log(logger, TraceLogger_FoldTests); FoldTests(graph); gs.spewPass("Fold Tests");