Ignore:
Timestamp:
Mar 1, 2021, 7:00:20 AM (4 years ago)
Author:
Alexey Shvayka
Message:

BytecodeGenerator::fuseCompareAndJump() fails for some language constructs
​https://bugs.webkit.org/show_bug.cgi?id=221927

Reviewed by Yusuke Suzuki.

For BytecodeGenerator::fuseCompareAndJump() to merge two ops into one, condition's dst
register should not be referenced from elsewhere. This change tracks down and eliminates
all such cases, which reduces bytecode size for a few language constructs:

-1 per every case of a switch;
-2 per generator function, -2 per every yield / yield*;
-2 per class extends;
-2 per finally, -1 per every break / continue / return inside;
-3 per Function.prototype.apply() with ...spread as a single argument.

Instead of mixing RefPtr with raw C++ pointers, single-line branches were preferred.
To keep them cleaner, this patch introduces emitLoad() override for JSGenerator::ResumeMode
enum, and tweaks existing override for CompletionType.

A few drive-by improvements:

  • to enable future optimizations, replaces emitBinaryOp() with emitEqualityOp() for OpEq / OpStricteq (adds an assert), and vice-versa for other comparison ops;
  • removes OperandTypes for comparison ops as it was ignored (let's re-introduce them consistently once supported);
  • inlines too specific BytecodeGenerator::emitJumpIf();
  • replaces eq with stricteq in ApplyFunctionCallDotNode.

No behavior change, no callee registers count grow.

  • bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::emitYield):
(JSC::BytecodeGenerator::emitDelegateYield):
(JSC::BytecodeGenerator::emitFinallyCompletion):
(JSC::BytecodeGenerator::emitJumpIf): Deleted.

  • bytecompiler/BytecodeGenerator.h:

(JSC::BytecodeGenerator::emitEqualityOp):
(JSC::BytecodeGenerator::emitLoad):

  • bytecompiler/NodesCodegen.cpp:

(JSC::ApplyFunctionCallDotNode::emitBytecode):
(JSC::ForInNode::emitBytecode):
(JSC::CaseBlockNode::emitBytecodeForBlock):
(JSC::FunctionNode::emitBytecode):
(JSC::ClassExprNode::emitBytecode):

Location:
trunk/Source/JavaScriptCore/bytecompiler
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r273225 r273649  
    47684768
    47694769    Ref<Label> normalLabel = newLabel();
    4770     RefPtr<RegisterID> condition = newTemporary();
    4771     emitEqualityOp<OpStricteq>(condition.get(), generatorResumeModeRegister(), emitLoad(nullptr, jsNumber(static_cast<int32_t>(JSGenerator::ResumeMode::NormalMode))));
    4772     emitJumpIfTrue(condition.get(), normalLabel.get());
     4770    emitJumpIfTrue(emitEqualityOp<OpStricteq>(newTemporary(), generatorResumeModeRegister(), emitLoad(nullptr, JSGenerator::ResumeMode::NormalMode)), normalLabel.get());
    47734771
    47744772    Ref<Label> throwLabel = newLabel();
    4775     emitEqualityOp<OpStricteq>(condition.get(), generatorResumeModeRegister(), emitLoad(nullptr, jsNumber(static_cast<int32_t>(JSGenerator::ResumeMode::ThrowMode))));
    4776     emitJumpIfTrue(condition.get(), throwLabel.get());
     4773    emitJumpIfTrue(emitEqualityOp<OpStricteq>(newTemporary(), generatorResumeModeRegister(), emitLoad(nullptr, JSGenerator::ResumeMode::ThrowMode)), throwLabel.get());
    47774774    // Return.
    47784775    {
    … …  
    49544951
    49554952                Ref<Label> normalLabel = newLabel();
     4953                emitJumpIfTrue(emitEqualityOp<OpStricteq>(newTemporary(), generatorResumeModeRegister(), emitLoad(nullptr, JSGenerator::ResumeMode::NormalMode)), normalLabel.get());
     4954
    49564955                Ref<Label> returnLabel = newLabel();
    4957                 {
    4958                     RefPtr<RegisterID> condition = newTemporary();
    4959                     emitEqualityOp<OpStricteq>(condition.get(), generatorResumeModeRegister(), emitLoad(nullptr, jsNumber(static_cast<int32_t>(JSGenerator::ResumeMode::NormalMode))));
    4960                     emitJumpIfTrue(condition.get(), normalLabel.get());
    4961 
    4962                     emitEqualityOp<OpStricteq>(condition.get(), generatorResumeModeRegister(), emitLoad(nullptr, jsNumber(static_cast<int32_t>(JSGenerator::ResumeMode::ReturnMode))));
    4963                     emitJumpIfTrue(condition.get(), returnLabel.get());
    4964 
    4965                     // Fallthrough to ThrowMode.
    4966                 }
     4956                emitJumpIfTrue(emitEqualityOp<OpStricteq>(newTemporary(), generatorResumeModeRegister(), emitLoad(nullptr, JSGenerator::ResumeMode::ReturnMode)), returnLabel.get());
    49674957
    49684958                // Throw.
    … …  
    51325122{
    51335123    if (context.numberOfBreaksOrContinues() || context.handlesReturns()) {
    5134         emitJumpIf<OpStricteq>(context.completionTypeRegister(), CompletionType::Normal, normalCompletionLabel);
     5124        emitJumpIfTrue(emitEqualityOp<OpStricteq>(newTemporary(), context.completionTypeRegister(), emitLoad(nullptr, CompletionType::Normal)), normalCompletionLabel);
    51355125
    51365126        FinallyContext* outerContext = context.outerContext();
    … …  
    51435133            Ref<Label> nextLabel = newLabel();
    51445134            auto& jump = context.jumps(i);
    5145             emitJumpIf<OpNstricteq>(context.completionTypeRegister(), jump.jumpID, nextLabel.get());
     5135            emitJumpIfFalse(emitEqualityOp<OpStricteq>(newTemporary(), context.completionTypeRegister(), emitLoad(nullptr, jump.jumpID)), nextLabel.get());
    51465136
    51475137            // This case is for Break / Continue completions from an inner finally context
    … …  
    51765166            if (context.handlesReturns()) {
    51775167                Ref<Label> isNotReturnLabel = newLabel();
    5178                 emitJumpIf<OpNstricteq>(context.completionTypeRegister(), CompletionType::Return, isNotReturnLabel.get());
     5168                emitJumpIfFalse(emitEqualityOp<OpStricteq>(newTemporary(), context.completionTypeRegister(), emitLoad(nullptr, CompletionType::Return)), isNotReturnLabel.get());
    51795169
    51805170                // This case is for Return completion from an inner finally context:
    … …  
    52045194            if (hasBreaksOrContinuesThatEscapeCurrentFinally) {
    52055195                Ref<Label> isThrowOrNormalLabel = newLabel();
    5206                 emitJumpIf<OpBeloweq>(context.completionTypeRegister(), CompletionType::Throw, isThrowOrNormalLabel.get());
     5196                emitJumpIfTrue(emitBinaryOp<OpBeloweq>(newTemporary(), context.completionTypeRegister(), emitLoad(nullptr, CompletionType::Throw)), isThrowOrNormalLabel.get());
    52075197
    52085198                // A completionType above Throw means we have a Break or Continue encoded as a jumpID.
    … …  
    52475237            if (context.handlesReturns()) {
    52485238                Ref<Label> notReturnLabel = newLabel();
    5249                 emitJumpIf<OpNstricteq>(context.completionTypeRegister(), CompletionType::Return, notReturnLabel.get());
     5239                emitJumpIfFalse(emitEqualityOp<OpStricteq>(newTemporary(), context.completionTypeRegister(), emitLoad(nullptr, CompletionType::Return)), notReturnLabel.get());
    52505240
    52515241                // This case is for Return completion from the outermost finally context:
    … …  
    52715261    // possibilities are Normal or Throw.
    52725262
    5273     emitJumpIf<OpNstricteq>(context.completionTypeRegister(), CompletionType::Throw, normalCompletionLabel);
     5263    emitJumpIfFalse(emitEqualityOp<OpStricteq>(newTemporary(), context.completionTypeRegister(), emitLoad(nullptr, CompletionType::Throw)), normalCompletionLabel);
    52745264
    52755265    // We get here because we entered this finally context with Throw completionType (i.e. we have
    … …  
    52845274
    52855275    emitThrow(context.completionValueRegister());
    5286 }
    5287 
    5288 template<typename CompareOp>
    5289 void BytecodeGenerator::emitJumpIf(RegisterID* completionTypeRegister, CompletionType type, Label& jumpTarget)
    5290 {
    5291     RefPtr<RegisterID> tempRegister = newTemporary();
    5292     RegisterID* valueConstant = addConstantValue(jsNumber(static_cast<int>(type)));
    5293     OperandTypes operandTypes = OperandTypes(ResultType::numberTypeIsInt32(), ResultType::unknownType());
    5294 
    5295     auto equivalenceResult = emitBinaryOp<CompareOp>(tempRegister.get(), completionTypeRegister, valueConstant, operandTypes);
    5296     emitJumpIfTrue(equivalenceResult, jumpTarget);
    52975276}
    52985277

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r273225 r273649  
    724724            && BinaryOp::opcodeID != op_div,
    725725            RegisterID*>
    726         emitBinaryOp(RegisterID* dst, RegisterID* src1, RegisterID* src2, OperandTypes)
     726        emitBinaryOp(RegisterID* dst, RegisterID* src1, RegisterID* src2, OperandTypes = OperandTypes())
    727727        {
    728728            BinaryOp::emit(this, dst, src1, src2);
    … …  
    748748        RegisterID* emitEqualityOp(RegisterID* dst, RegisterID* src1, RegisterID* src2)
    749749        {
     750            static_assert(EqOp::opcodeID == op_eq || EqOp::opcodeID == op_stricteq);
    750751            if (!emitEqualityOpImpl(dst, src1, src2))
    751752                EqOp::emit(this, dst, src1, src2);
    … …  
    10211022        void emitWillLeaveCallFrameDebugHook();
    10221023
    1023         void emitLoad(RegisterID* completionTypeRegister, CompletionType type)
    1024         {
    1025             emitLoad(completionTypeRegister, JSValue(static_cast<int>(type)));
    1026         }
    1027 
    1028         template<typename CompareOp>
    1029         void emitJumpIf(RegisterID* completionTypeRegister, CompletionType, Label& jumpTarget);
     1024        RegisterID* emitLoad(RegisterID* dst, CompletionType type) { return emitLoad(dst, jsNumber(static_cast<int32_t>(type))); }
     1025        RegisterID* emitLoad(RegisterID* dst, JSGenerator::ResumeMode mode) { return emitLoad(dst, jsNumber(static_cast<int32_t>(mode))); }
    10301026
    10311027        bool emitJumpViaFinallyIfNeeded(int targetLabelScopeDepth, Label& jumpTarget);

  • trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

    r273225 r273649  
    22452245                    Ref<Label> haveThis = generator.newLabel();
    22462246                    Ref<Label> end = generator.newLabel();
    2247                     RefPtr<RegisterID> compareResult = generator.newTemporary();
    2248                     RefPtr<RegisterID> indexZeroCompareResult = generator.emitBinaryOp<OpEq>(compareResult.get(), index.get(), generator.emitLoad(nullptr, jsNumber(0)), OperandTypes(ResultType::numberTypeIsInt32(), ResultType::numberTypeIsInt32()));
    2249                     generator.emitJumpIfFalse(indexZeroCompareResult.get(), haveThis.get());
     2247                    generator.emitJumpIfFalse(generator.emitEqualityOp<OpStricteq>(generator.newTemporary(), index.get(), generator.emitLoad(nullptr, jsNumber(0))), haveThis.get());
    22502248                    generator.move(thisRegister.get(), value);
    22512249                    generator.emitLoad(index.get(), jsNumber(1));
    22522250                    generator.emitJump(end.get());
    22532251                    generator.emitLabel(haveThis.get());
    2254                     RefPtr<RegisterID> indexOneCompareResult = generator.emitBinaryOp<OpEq>(compareResult.get(), index.get(), generator.emitLoad(nullptr, jsNumber(1)), OperandTypes(ResultType::numberTypeIsInt32(), ResultType::numberTypeIsInt32()));
    2255                     generator.emitJumpIfFalse(indexOneCompareResult.get(), end.get());
     2252                    generator.emitJumpIfFalse(generator.emitEqualityOp<OpStricteq>(generator.newTemporary(), index.get(), generator.emitLoad(nullptr, jsNumber(1))), end.get());
    22562253                    generator.move(argumentsRegister.get(), value);
    22572254                    generator.emitLoad(index.get(), jsNumber(2));
    … …  
    42134210        generator.emitLoopHint();
    42144211
    4215         RefPtr<RegisterID> result = generator.emitEqualityOp<OpLess>(generator.newTemporary(), i.get(), length.get());
     4212        RefPtr<RegisterID> result = generator.emitBinaryOp<OpLess>(generator.newTemporary(), i.get(), length.get());
    42164213        generator.emitJumpIfFalse(result.get(), loopEnd.get());
    42174214        generator.emitHasEnumerableIndexedProperty(result.get(), base.get(), i.get());
    … …  
    46194616            RefPtr<RegisterID> clauseVal = generator.newTemporary();
    46204617            generator.emitNode(clauseVal.get(), list->getClause()->expr());
    4621             generator.emitBinaryOp<OpStricteq>(clauseVal.get(), clauseVal.get(), switchExpression, OperandTypes());
    4622             labelVector.append(generator.newLabel());
    4623             generator.emitJumpIfTrue(clauseVal.get(), labelVector[labelVector.size() - 1].get());
     4618            Ref<Label> clauseLabel = generator.newLabel();
     4619            labelVector.append(clauseLabel);
     4620            generator.emitJumpIfTrue(generator.emitEqualityOp<OpStricteq>(generator.newTemporary(), clauseVal.get(), switchExpression), clauseLabel.get());
    46244621        }
    46254622       
    … …  
    46274624            RefPtr<RegisterID> clauseVal = generator.newTemporary();
    46284625            generator.emitNode(clauseVal.get(), list->getClause()->expr());
    4629             generator.emitBinaryOp<OpStricteq>(clauseVal.get(), clauseVal.get(), switchExpression, OperandTypes());
    4630             labelVector.append(generator.newLabel());
    4631             generator.emitJumpIfTrue(clauseVal.get(), labelVector[labelVector.size() - 1].get());
     4626            Ref<Label> clauseLabel = generator.newLabel();
     4627            labelVector.append(clauseLabel);
     4628            generator.emitJumpIfTrue(generator.emitEqualityOp<OpStricteq>(generator.newTemporary(), clauseVal.get(), switchExpression), clauseLabel.get());
    46324629        }
    46334630        generator.emitJump(defaultLabel.get());
    … …  
    49644961        generator.move(args.argumentRegister(argumentCount++), generator.promiseRegister());
    49654962        generator.emitLoad(args.argumentRegister(argumentCount++), jsUndefined());
    4966         generator.emitLoad(args.argumentRegister(argumentCount++), jsNumber(static_cast<int32_t>(JSGenerator::ResumeMode::NormalMode)));
     4963        generator.emitLoad(args.argumentRegister(argumentCount++), JSGenerator::ResumeMode::NormalMode);
    49674964        // JSTextPosition(int _line, int _offset, int _lineStartOffset)
    49684965        JSTextPosition divot(firstLine(), startOffset(), lineStartOffset());
    … …  
    49804977        Ref<Label> generatorBodyLabel = generator.newLabel();
    49814978        {
    4982             RefPtr<RegisterID> condition = generator.newTemporary();
    4983             generator.emitEqualityOp<OpStricteq>(condition.get(), generator.generatorResumeModeRegister(), generator.emitLoad(nullptr, jsNumber(static_cast<int32_t>(JSGenerator::ResumeMode::NormalMode))));
    4984             generator.emitJumpIfTrue(condition.get(), generatorBodyLabel.get());
     4979            generator.emitJumpIfTrue(generator.emitEqualityOp<OpStricteq>(generator.newTemporary(), generator.generatorResumeModeRegister(), generator.emitLoad(nullptr, JSGenerator::ResumeMode::NormalMode)), generatorBodyLabel.get());
    49854980
    49864981            Ref<Label> throwLabel = generator.newLabel();
    4987             generator.emitEqualityOp<OpStricteq>(condition.get(), generator.generatorResumeModeRegister(), generator.emitLoad(nullptr, jsNumber(static_cast<int32_t>(JSGenerator::ResumeMode::ThrowMode))));
    4988             generator.emitJumpIfTrue(condition.get(), throwLabel.get());
     4982            generator.emitJumpIfTrue(generator.emitEqualityOp<OpStricteq>(generator.newTemporary(), generator.generatorResumeModeRegister(), generator.emitLoad(nullptr, JSGenerator::ResumeMode::ThrowMode)), throwLabel.get());
    49894983
    49904984            generator.emitReturn(generator.generatorValueRegister());
    … …  
    52135207        generator.emitLoad(protoParent.get(), jsNull());
    52145208
    5215         RefPtr<RegisterID> tempRegister = generator.newTemporary();
    5216 
    52175209        Ref<Label> superclassIsNullLabel = generator.newLabel();
    5218         generator.emitJumpIfTrue(generator.emitIsNull(tempRegister.get(), superclass.get()), superclassIsNullLabel.get());
     5210        generator.emitJumpIfTrue(generator.emitIsNull(generator.newTemporary(), superclass.get()), superclassIsNullLabel.get());
    52195211
    52205212        Ref<Label> superclassIsConstructorLabel = generator.newLabel();
    5221         generator.emitJumpIfTrue(generator.emitIsConstructor(tempRegister.get(), superclass.get()), superclassIsConstructorLabel.get());
     5213        generator.emitJumpIfTrue(generator.emitIsConstructor(generator.newTemporary(), superclass.get()), superclassIsConstructorLabel.get());
    52225214        generator.emitThrowTypeError("The superclass is not a constructor."_s);
    52235215        generator.emitLabel(superclassIsConstructorLabel.get());
    … …  
    52255217
    52265218        Ref<Label> protoParentIsObjectOrNullLabel = generator.newLabel();
    5227         generator.emitJumpIfTrue(generator.emitIsObject(tempRegister.get(), protoParent.get()), protoParentIsObjectOrNullLabel.get());
    5228         generator.emitJumpIfTrue(generator.emitIsNull(tempRegister.get(), protoParent.get()), protoParentIsObjectOrNullLabel.get());
     5219        generator.emitJumpIfTrue(generator.emitIsObject(generator.newTemporary(), protoParent.get()), protoParentIsObjectOrNullLabel.get());
     5220        generator.emitJumpIfTrue(generator.emitIsNull(generator.newTemporary(), protoParent.get()), protoParentIsObjectOrNullLabel.get());
    52295221        generator.emitThrowTypeError("The value of the superclass's prototype property is not an object or null."_s);
    52305222        generator.emitLabel(protoParentIsObjectOrNullLabel.get());
Note: See TracChangeset for help on using the changeset viewer.