From b015229c091cb064c101b28b8da477ef1a356bd6 Mon Sep 17 00:00:00 2001 From: "rodolph.perfetta" Date: Fri, 2 Jun 2017 09:17:42 -0700 Subject: [PATCH] handle WASM trap in the instruction scheduler. Review-Url: https://codereview.chromium.org/2916143003 Cr-Commit-Position: refs/heads/master@{#45694} --- .../ia32/instruction-scheduler-ia32.cc | 4 +- src/compiler/instruction-scheduler.cc | 22 +++++------ src/compiler/instruction-scheduler.h | 37 +++++++++++-------- src/compiler/instruction.h | 4 ++ src/compiler/x64/instruction-scheduler-x64.cc | 4 +- 5 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/compiler/ia32/instruction-scheduler-ia32.cc b/src/compiler/ia32/instruction-scheduler-ia32.cc index 64aeed9b25..54efb3e45e 100644 --- a/src/compiler/ia32/instruction-scheduler-ia32.cc +++ b/src/compiler/ia32/instruction-scheduler-ia32.cc @@ -111,8 +111,8 @@ int InstructionScheduler::GetTargetInstructionFlags( case kIA32Idiv: case kIA32Udiv: return (instr->addressing_mode() == kMode_None) - ? kMayNeedDeoptCheck - : kMayNeedDeoptCheck | kIsLoadOperation | kHasSideEffect; + ? kMayNeedDeoptOrTrapCheck + : kMayNeedDeoptOrTrapCheck | kIsLoadOperation | kHasSideEffect; case kIA32Movsxbl: case kIA32Movzxbl: diff --git a/src/compiler/instruction-scheduler.cc b/src/compiler/instruction-scheduler.cc index 83d163775d..e311abb2a2 100644 --- a/src/compiler/instruction-scheduler.cc +++ b/src/compiler/instruction-scheduler.cc @@ -77,7 +77,6 @@ void InstructionScheduler::ScheduleGraphNode::AddSuccessor( node->unscheduled_predecessors_count_++; } - InstructionScheduler::InstructionScheduler(Zone* zone, InstructionSequence* sequence) : zone_(zone), @@ -86,16 +85,15 @@ InstructionScheduler::InstructionScheduler(Zone* zone, last_side_effect_instr_(nullptr), pending_loads_(zone), last_live_in_reg_marker_(nullptr), - last_deopt_(nullptr), + last_deopt_or_trap_(nullptr), operands_map_(zone) {} - void InstructionScheduler::StartBlock(RpoNumber rpo) { DCHECK(graph_.empty()); DCHECK(last_side_effect_instr_ == nullptr); DCHECK(pending_loads_.empty()); DCHECK(last_live_in_reg_marker_ == nullptr); - DCHECK(last_deopt_ == nullptr); + DCHECK(last_deopt_or_trap_ == nullptr); DCHECK(operands_map_.empty()); sequence()->StartBlock(rpo); } @@ -112,7 +110,7 @@ void InstructionScheduler::EndBlock(RpoNumber rpo) { last_side_effect_instr_ = nullptr; pending_loads_.clear(); last_live_in_reg_marker_ = nullptr; - last_deopt_ = nullptr; + last_deopt_or_trap_ = nullptr; operands_map_.clear(); } @@ -137,9 +135,9 @@ void InstructionScheduler::AddInstruction(Instruction* instr) { } // Make sure that instructions are not scheduled before the last - // deoptimization point when they depend on it. - if ((last_deopt_ != nullptr) && DependsOnDeoptimization(instr)) { - last_deopt_->AddSuccessor(new_node); + // deoptimization or trap point when they depend on it. + if ((last_deopt_or_trap_ != nullptr) && DependsOnDeoptOrTrap(instr)) { + last_deopt_or_trap_->AddSuccessor(new_node); } // Instructions with side effects and memory operations can't be @@ -160,13 +158,13 @@ void InstructionScheduler::AddInstruction(Instruction* instr) { last_side_effect_instr_->AddSuccessor(new_node); } pending_loads_.push_back(new_node); - } else if (instr->IsDeoptimizeCall()) { - // Ensure that deopts are not reordered with respect to side-effect - // instructions. + } else if (instr->IsDeoptimizeCall() || instr->IsTrap()) { + // Ensure that deopts or traps are not reordered with respect to + // side-effect instructions. if (last_side_effect_instr_ != nullptr) { last_side_effect_instr_->AddSuccessor(new_node); } - last_deopt_ = new_node; + last_deopt_or_trap_ = new_node; } // Look for operand dependencies. diff --git a/src/compiler/instruction-scheduler.h b/src/compiler/instruction-scheduler.h index 7660520b6d..db2894a92a 100644 --- a/src/compiler/instruction-scheduler.h +++ b/src/compiler/instruction-scheduler.h @@ -21,10 +21,11 @@ enum ArchOpcodeFlags { kHasSideEffect = 2, // The instruction has some side effects (memory // store, function call...) kIsLoadOperation = 4, // The instruction is a memory load. - kMayNeedDeoptCheck = 8, // The instruction might be associated with a deopt - // check. This is the case of instruction which can - // blow up with particular inputs (e.g.: division by - // zero on Intel platforms). + kMayNeedDeoptOrTrapCheck = 8, // The instruction may be associated with a + // deopt or trap check which must be run before + // instruction e.g. div on Intel platform which + // will raise an exception when the divisor is + // zero. }; class InstructionScheduler final : public ZoneObject { @@ -166,17 +167,22 @@ class InstructionScheduler final : public ZoneObject { return (GetInstructionFlags(instr) & kIsLoadOperation) != 0; } - // Return true if this instruction is usually associated with a deopt check - // to validate its input. - bool MayNeedDeoptCheck(const Instruction* instr) const { - return (GetInstructionFlags(instr) & kMayNeedDeoptCheck) != 0; + // The scheduler will not move the following instructions before the last + // deopt/trap check: + // * loads (this is conservative) + // * instructions with side effect + // * other deopts/traps + // Any other instruction can be moved, apart from those that raise exceptions + // on specific inputs - these are filtered out by the deopt/trap check. + bool MayNeedDeoptOrTrapCheck(const Instruction* instr) const { + return (GetInstructionFlags(instr) & kMayNeedDeoptOrTrapCheck) != 0; } - // Return true if the instruction cannot be moved before the last deopt - // point we encountered. - bool DependsOnDeoptimization(const Instruction* instr) const { - return MayNeedDeoptCheck(instr) || instr->IsDeoptimizeCall() || - HasSideEffect(instr) || IsLoadOperation(instr); + // Return true if the instruction cannot be moved before the last deopt or + // trap point we encountered. + bool DependsOnDeoptOrTrap(const Instruction* instr) const { + return MayNeedDeoptOrTrapCheck(instr) || instr->IsDeoptimizeCall() || + instr->IsTrap() || HasSideEffect(instr) || IsLoadOperation(instr); } // Identify nops used as a definition point for live-in registers at @@ -217,8 +223,9 @@ class InstructionScheduler final : public ZoneObject { // other instructions in the basic block. ScheduleGraphNode* last_live_in_reg_marker_; - // Last deoptimization instruction encountered while building the graph. - ScheduleGraphNode* last_deopt_; + // Last deoptimization or trap instruction encountered while building the + // graph. + ScheduleGraphNode* last_deopt_or_trap_; // Keep track of definition points for virtual registers. This is used to // record operand dependencies in the scheduling graph. diff --git a/src/compiler/instruction.h b/src/compiler/instruction.h index bba5bb3c17..cc19c52816 100644 --- a/src/compiler/instruction.h +++ b/src/compiler/instruction.h @@ -897,6 +897,10 @@ class V8_EXPORT_PRIVATE Instruction final { FlagsModeField::decode(opcode()) == kFlags_deoptimize; } + bool IsTrap() const { + return FlagsModeField::decode(opcode()) == kFlags_trap; + } + bool IsJump() const { return arch_opcode() == ArchOpcode::kArchJmp; } bool IsRet() const { return arch_opcode() == ArchOpcode::kArchRet; } bool IsTailCall() const { diff --git a/src/compiler/x64/instruction-scheduler-x64.cc b/src/compiler/x64/instruction-scheduler-x64.cc index 862bca21a1..63f134c081 100644 --- a/src/compiler/x64/instruction-scheduler-x64.cc +++ b/src/compiler/x64/instruction-scheduler-x64.cc @@ -189,8 +189,8 @@ int InstructionScheduler::GetTargetInstructionFlags( case kX64Udiv: case kX64Udiv32: return (instr->addressing_mode() == kMode_None) - ? kMayNeedDeoptCheck - : kMayNeedDeoptCheck | kIsLoadOperation | kHasSideEffect; + ? kMayNeedDeoptOrTrapCheck + : kMayNeedDeoptOrTrapCheck | kIsLoadOperation | kHasSideEffect; case kX64Movsxbl: case kX64Movzxbl: