[wasm] move protected instructions from RelocInfo To FixedArray

The motivation for this is that it greatly reduces the RelocInfo size.
This also results in a small improvement in compile time.

Note: This CL was based on https://codereview.chromium.org/2651833003,
and basically reverts that CL (but handles code changes and some
minor bugs in previous code).

Bug: chromium:772780
Change-Id: I55dd48d3bddd4b3d1c8eec13791b3ee4c485c604
Reviewed-on: https://chromium-review.googlesource.com/730649Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Eric Holk <eholk@chromium.org>
Commit-Queue: Karl Schimpf <kschimpf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48947}

src/assembler.cc

@@ -431,8 +431,7 @@ void RelocInfoWriter::Write(const RelocInfo* rinfo) {
       WriteData(rinfo->data());
     } else if (RelocInfo::IsConstPool(rmode) ||
                RelocInfo::IsVeneerPool(rmode) || RelocInfo::IsDeoptId(rmode) ||
-               RelocInfo::IsDeoptPosition(rmode) ||
-               RelocInfo::IsWasmProtectedLanding(rmode)) {
+               RelocInfo::IsDeoptPosition(rmode)) {
       WriteIntData(static_cast<int>(rinfo->data()));
     }
   }
@@ -534,8 +533,7 @@ void RelocIterator::next() {
         } else if (RelocInfo::IsConstPool(rmode) ||
                    RelocInfo::IsVeneerPool(rmode) ||
                    RelocInfo::IsDeoptId(rmode) ||
-                   RelocInfo::IsDeoptPosition(rmode) ||
-                   RelocInfo::IsWasmProtectedLanding(rmode)) {
+                   RelocInfo::IsDeoptPosition(rmode)) {
           if (SetMode(rmode)) {
             AdvanceReadInt();
             return;
@@ -628,8 +626,6 @@ const char* RelocInfo::RelocModeName(RelocInfo::Mode rmode) {
       return "wasm context reference";
     case WASM_FUNCTION_TABLE_SIZE_REFERENCE:
       return "wasm function table size reference";
-    case WASM_PROTECTED_INSTRUCTION_LANDING:
-      return "wasm protected instruction landing";
     case WASM_GLOBAL_HANDLE:
       return "global handle";
     case NUMBER_OF_MODES:
@@ -713,8 +709,6 @@ void RelocInfo::Verify(Isolate* isolate) {
     case WASM_CONTEXT_REFERENCE:
     case WASM_FUNCTION_TABLE_SIZE_REFERENCE:
     case WASM_GLOBAL_HANDLE:
-    case WASM_PROTECTED_INSTRUCTION_LANDING:
-    // TODO(eholk): make sure the protected instruction is in range.
     case NONE32:
     case NONE64:
       break;

src/assembler.h

@@ -365,7 +365,6 @@ class RelocInfo {
     // GC'ed.
     WASM_CONTEXT_REFERENCE,
     WASM_FUNCTION_TABLE_SIZE_REFERENCE,
-    WASM_PROTECTED_INSTRUCTION_LANDING,
     WASM_GLOBAL_HANDLE,
 
     RUNTIME_ENTRY,
@@ -471,9 +470,6 @@ class RelocInfo {
   static inline bool IsWasmPtrReference(Mode mode) {
     return mode == WASM_CONTEXT_REFERENCE || mode == WASM_GLOBAL_HANDLE;
   }
-  static inline bool IsWasmProtectedLanding(Mode mode) {
-    return mode == WASM_PROTECTED_INSTRUCTION_LANDING;
-  }
 
   static inline int ModeMask(Mode mode) { return 1 << mode; }
 

src/compiler/code-generator.cc

@@ -36,11 +36,12 @@ class CodeGenerator::JumpTable final : public ZoneObject {
   size_t const target_count_;
 };
 
-CodeGenerator::CodeGenerator(Zone* codegen_zone, Frame* frame, Linkage* linkage,
-                             InstructionSequence* code, CompilationInfo* info,
-                             base::Optional<OsrHelper> osr_helper,
-                             int start_source_position,
-                             JumpOptimizationInfo* jump_opt)
+CodeGenerator::CodeGenerator(
+    Zone* codegen_zone, Frame* frame, Linkage* linkage,
+    InstructionSequence* code, CompilationInfo* info,
+    base::Optional<OsrHelper> osr_helper, int start_source_position,
+    JumpOptimizationInfo* jump_opt,
+    std::vector<trap_handler::ProtectedInstructionData>* protected_instructions)
     : zone_(codegen_zone),
       frame_access_state_(nullptr),
       linkage_(linkage),
@@ -69,6 +70,7 @@ CodeGenerator::CodeGenerator(Zone* codegen_zone, Frame* frame, Linkage* linkage,
       optimized_out_literal_id_(-1),
       source_position_table_builder_(zone(),
                                      info->SourcePositionRecordingMode()),
+      protected_instructions_(protected_instructions),
       result_(kSuccess) {
   for (int i = 0; i < code->InstructionBlockCount(); ++i) {
     new (&labels_[i]) Label;
@@ -80,6 +82,15 @@ CodeGenerator::CodeGenerator(Zone* codegen_zone, Frame* frame, Linkage* linkage,
 
 Isolate* CodeGenerator::isolate() const { return info_->isolate(); }
 
+void CodeGenerator::AddProtectedInstructionLanding(uint32_t instr_offset,
+                                                   uint32_t landing_offset) {
+  if (protected_instructions_ != nullptr) {
+    trap_handler::ProtectedInstructionData data = {instr_offset,
+                                                   landing_offset};
+    protected_instructions_->emplace_back(data);
+  }
+}
+
 void CodeGenerator::CreateFrameAccessState(Frame* frame) {
   FinishFrame(frame);
   frame_access_state_ = new (zone()) FrameAccessState(frame);

src/compiler/code-generator.h

@@ -81,7 +81,9 @@ class CodeGenerator final : public GapResolver::Assembler {
                          InstructionSequence* code, CompilationInfo* info,
                          base::Optional<OsrHelper> osr_helper,
                          int start_source_position,
-                         JumpOptimizationInfo* jump_opt);
+                         JumpOptimizationInfo* jump_opt,
+                         std::vector<trap_handler::ProtectedInstructionData>*
+                             protected_instructions);
 
   // Generate native code. After calling AssembleCode, call FinalizeCode to
   // produce the actual code object. If an error occurs during either phase,
@@ -97,6 +99,9 @@ class CodeGenerator final : public GapResolver::Assembler {
 
   Label* GetLabel(RpoNumber rpo) { return &labels_[rpo.ToSize()]; }
 
+  void AddProtectedInstructionLanding(uint32_t instr_offset,
+                                      uint32_t landing_offset);
+
   SourcePosition start_source_position() const {
     return start_source_position_;
   }
@@ -353,6 +358,7 @@ class CodeGenerator final : public GapResolver::Assembler {
   int osr_pc_offset_;
   int optimized_out_literal_id_;
   SourcePositionTableBuilder source_position_table_builder_;
+  std::vector<trap_handler::ProtectedInstructionData>* protected_instructions_;
   CodeGenResult result_;
 };
 

src/compiler/pipeline.cc

@@ -116,7 +116,7 @@ class PipelineData {
   PipelineData(ZoneStats* zone_stats, CompilationInfo* info, JSGraph* jsgraph,
                PipelineStatistics* pipeline_statistics,
                SourcePositionTable* source_positions,
-               ZoneVector<trap_handler::ProtectedInstructionData>*
+               std::vector<trap_handler::ProtectedInstructionData>*
                    protected_instructions)
       : isolate_(info->isolate()),
         info_(info),
@@ -136,8 +136,7 @@ class PipelineData {
         codegen_zone_(codegen_zone_scope_.zone()),
         register_allocation_zone_scope_(zone_stats_, ZONE_NAME),
         register_allocation_zone_(register_allocation_zone_scope_.zone()),
-        protected_instructions_(protected_instructions) {
-  }
+        protected_instructions_(protected_instructions) {}
 
   // For machine graph testing entry point.
   PipelineData(ZoneStats* zone_stats, CompilationInfo* info, Graph* graph,
@@ -249,7 +248,7 @@ class PipelineData {
     source_position_output_ = source_position_output;
   }
 
-  ZoneVector<trap_handler::ProtectedInstructionData>* protected_instructions()
+  std::vector<trap_handler::ProtectedInstructionData>* protected_instructions()
       const {
     return protected_instructions_;
   }
@@ -337,9 +336,10 @@ class PipelineData {
 
   void InitializeCodeGenerator(Linkage* linkage) {
     DCHECK_NULL(code_generator_);
-    code_generator_ = new CodeGenerator(
-        codegen_zone(), frame(), linkage, sequence(), info(), osr_helper_,
-        start_source_position_, jump_optimization_info_);
+    code_generator_ =
+        new CodeGenerator(codegen_zone(), frame(), linkage, sequence(), info(),
+                          osr_helper_, start_source_position_,
+                          jump_optimization_info_, protected_instructions_);
   }
 
   void BeginPhaseKind(const char* phase_kind_name) {
@@ -410,7 +410,7 @@ class PipelineData {
   // Source position output for --trace-turbo.
   std::string source_position_output_;
 
-  ZoneVector<trap_handler::ProtectedInstructionData>* protected_instructions_ =
+  std::vector<trap_handler::ProtectedInstructionData>* protected_instructions_ =
       nullptr;
 
   JumpOptimizationInfo* jump_optimization_info_ = nullptr;
@@ -891,7 +891,7 @@ class PipelineWasmCompilationJob final : public CompilationJob {
   explicit PipelineWasmCompilationJob(
       CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor,
       SourcePositionTable* source_positions,
-      ZoneVector<trap_handler::ProtectedInstructionData>* protected_insts,
+      std::vector<trap_handler::ProtectedInstructionData>* protected_insts,
       bool asmjs_origin)
       : CompilationJob(info->isolate()->stack_guard()->real_climit(), nullptr,
                        info, "TurboFan", State::kReadyToExecute),
@@ -2037,7 +2037,7 @@ CompilationJob* Pipeline::NewCompilationJob(Handle<JSFunction> function,
 CompilationJob* Pipeline::NewWasmCompilationJob(
     CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor,
     SourcePositionTable* source_positions,
-    ZoneVector<trap_handler::ProtectedInstructionData>* protected_instructions,
+    std::vector<trap_handler::ProtectedInstructionData>* protected_instructions,
     wasm::ModuleOrigin asmjs_origin) {
   return new PipelineWasmCompilationJob(info, jsgraph, descriptor,
                                         source_positions,

src/compiler/pipeline.h

@@ -47,7 +47,7 @@ class Pipeline : public AllStatic {
   static CompilationJob* NewWasmCompilationJob(
       CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor,
       SourcePositionTable* source_positions,
-      ZoneVector<trap_handler::ProtectedInstructionData>*
+      std::vector<trap_handler::ProtectedInstructionData>*
           protected_instructions,
       wasm::ModuleOrigin wasm_origin);
 

src/compiler/wasm-compiler.cc

@@ -4789,12 +4789,10 @@ void WasmCompilationUnit::ExecuteTurbofanCompilation() {
     tf_.info_.reset(new CompilationInfo(
         GetDebugName(tf_.compilation_zone_.get(), func_name_, func_index_),
         isolate_, tf_.compilation_zone_.get(), Code::WASM_FUNCTION));
-    ZoneVector<trap_handler::ProtectedInstructionData> protected_instructions(
-        tf_.compilation_zone_.get());
 
     tf_.job_.reset(Pipeline::NewWasmCompilationJob(
         tf_.info_.get(), tf_.jsgraph_, descriptor, source_positions,
-        &protected_instructions, env_->module->origin()));
+        &protected_instructions_, env_->module->origin()));
     ok_ = tf_.job_->ExecuteJob() == CompilationJob::SUCCEEDED;
     // TODO(bradnelson): Improve histogram handling of size_t.
     counters()->wasm_compile_function_peak_memory_bytes()->AddSample(
@@ -4873,9 +4871,27 @@ MaybeHandle<Code> WasmCompilationUnit::FinishTurbofanCompilation(
            codegen_ms);
   }
 
+  PackProtectedInstructions(code);
   return code;
 }
 
+void WasmCompilationUnit::PackProtectedInstructions(Handle<Code> code) const {
+  if (protected_instructions_.empty()) return;
+  DCHECK_LT(protected_instructions_.size(), std::numeric_limits<int>::max());
+  const int num_instructions = static_cast<int>(protected_instructions_.size());
+  Handle<FixedArray> fn_protected = isolate_->factory()->NewFixedArray(
+      num_instructions * Code::kTrapDataSize, TENURED);
+  for (int i = 0; i < num_instructions; ++i) {
+    const trap_handler::ProtectedInstructionData& instruction =
+        protected_instructions_[i];
+    fn_protected->set(Code::kTrapDataSize * i + Code::kTrapCodeOffset,
+                      Smi::FromInt(instruction.instr_offset));
+    fn_protected->set(Code::kTrapDataSize * i + Code::kTrapLandingOffset,
+                      Smi::FromInt(instruction.landing_offset));
+  }
+  code->set_protected_instructions(*fn_protected);
+}
+
 MaybeHandle<Code> WasmCompilationUnit::FinishLiftoffCompilation(
     wasm::ErrorThrower* thrower) {
   CodeDesc desc;
@@ -4900,6 +4916,7 @@ MaybeHandle<Code> WasmCompilationUnit::FinishLiftoffCompilation(
                               "wasm#%d-liftoff", func_index_);
   }
 
+  PackProtectedInstructions(code);
   return code;
 }
 

src/compiler/wasm-compiler.h

@@ -103,6 +103,8 @@ class WasmCompilationUnit final {
   size_t memory_cost() const { return memory_cost_; }
 
  private:
+  void PackProtectedInstructions(Handle<Code> code) const;
+
   struct LiftoffData {
     wasm::LiftoffAssembler asm_;
     explicit LiftoffData(Isolate* isolate) : asm_(isolate) {}
@@ -141,6 +143,7 @@ class WasmCompilationUnit final {
   bool ok_ = true;
   size_t memory_cost_ = 0;
   bool lower_simd_;
+  std::vector<trap_handler::ProtectedInstructionData> protected_instructions_;
   CompilationMode mode_;
   // {liftoff_} is valid if mode_ == kLiftoff, tf_ if mode_ == kTurbofan.
   union {

src/compiler/x64/code-generator-x64.cc

@@ -288,7 +288,7 @@ class WasmOutOfLineTrap final : public OutOfLineCode {
   // TODO(eholk): Refactor this method to take the code generator as a
   // parameter.
   void Generate() final {
-    __ RecordProtectedInstructionLanding(pc_);
+    gen_->AddProtectedInstructionLanding(pc_, __ pc_offset());
 
     if (frame_elided_) {
       __ EnterFrame(StackFrame::WASM_COMPILED);

src/factory.cc

@@ -1847,6 +1847,7 @@ Handle<Code> Factory::NewCode(
   code->set_next_code_link(*undefined_value(), SKIP_WRITE_BARRIER);
   code->set_handler_table(*handler_table);
   code->set_source_position_table(*source_position_table);
+  code->set_protected_instructions(*empty_fixed_array(), SKIP_WRITE_BARRIER);
   code->set_constant_pool_offset(desc.instr_size - desc.constant_pool_size);
   code->set_builtin_index(-1);
   code->set_trap_handler_index(Smi::FromInt(-1));

src/objects-body-descriptors-inl.h

@@ -355,6 +355,8 @@ class Code::BodyDescriptor final : public BodyDescriptorBase {
   STATIC_ASSERT(kDeoptimizationDataOffset + kPointerSize ==
                 kSourcePositionTableOffset);
   STATIC_ASSERT(kSourcePositionTableOffset + kPointerSize ==
+                kProtectedInstructionsOffset);
+  STATIC_ASSERT(kProtectedInstructionsOffset + kPointerSize ==
                 kCodeDataContainerOffset);
   STATIC_ASSERT(kCodeDataContainerOffset + kPointerSize == kNextCodeLinkOffset);
 

src/objects/code-inl.h

@@ -157,6 +157,7 @@ CODE_ACCESSORS(relocation_info, ByteArray, kRelocationInfoOffset)
 CODE_ACCESSORS(handler_table, FixedArray, kHandlerTableOffset)
 CODE_ACCESSORS(deoptimization_data, FixedArray, kDeoptimizationDataOffset)
 CODE_ACCESSORS(source_position_table, Object, kSourcePositionTableOffset)
+CODE_ACCESSORS(protected_instructions, FixedArray, kProtectedInstructionsOffset)
 CODE_ACCESSORS(code_data_container, CodeDataContainer, kCodeDataContainerOffset)
 CODE_ACCESSORS(trap_handler_index, Smi, kTrapHandlerIndex)
 CODE_ACCESSORS(next_code_link, Object, kNextCodeLinkOffset)
@@ -167,6 +168,7 @@ void Code::WipeOutHeader() {
   WRITE_FIELD(this, kHandlerTableOffset, nullptr);
   WRITE_FIELD(this, kDeoptimizationDataOffset, nullptr);
   WRITE_FIELD(this, kSourcePositionTableOffset, nullptr);
+  WRITE_FIELD(this, kProtectedInstructionsOffset, nullptr);
   WRITE_FIELD(this, kCodeDataContainerOffset, nullptr);
   WRITE_FIELD(this, kNextCodeLinkOffset, nullptr);
 }
@@ -245,6 +247,7 @@ int Code::SizeIncludingMetadata() const {
   size += relocation_info()->Size();
   size += deoptimization_data()->Size();
   size += handler_table()->Size();
+  size += protected_instructions()->Size();
   return size;
 }
 

src/objects/code.h

@@ -160,6 +160,10 @@ class Code : public HeapObject {
   DECL_ACCESSORS(source_position_table, Object)
   inline ByteArray* SourcePositionTable() const;
 
+  // [protected instructions]: Array containing list of protected
+  // instructions and corresponding landing pad offset.
+  DECL_ACCESSORS(protected_instructions, FixedArray)
+
   // [code_data_container]: A container indirection for all mutable fields.
   DECL_ACCESSORS(code_data_container, CodeDataContainer)
 
@@ -431,8 +435,10 @@ class Code : public HeapObject {
       kHandlerTableOffset + kPointerSize;
   static const int kSourcePositionTableOffset =
       kDeoptimizationDataOffset + kPointerSize;
-  static const int kCodeDataContainerOffset =
+  static const int kProtectedInstructionsOffset =
       kSourcePositionTableOffset + kPointerSize;
+  static const int kCodeDataContainerOffset =
+      kProtectedInstructionsOffset + kPointerSize;
   static const int kNextCodeLinkOffset =
       kCodeDataContainerOffset + kPointerSize;
   static const int kInstructionSizeOffset = kNextCodeLinkOffset + kPointerSize;
@@ -455,6 +461,8 @@ class Code : public HeapObject {
   // Objects embedded into code is visited via reloc info.
   static const int kDataStart = kInstructionSizeOffset;
 
+  enum TrapFields { kTrapCodeOffset, kTrapLandingOffset, kTrapDataSize };
+
   inline int GetUnwindingInfoSizeOffset() const;
 
   class BodyDescriptor;

src/wasm/wasm-module.cc

@@ -76,22 +76,29 @@ void UnpackAndRegisterProtectedInstructions(Isolate* isolate,
 
     byte* base = code->entry();
 
-    const int mode_mask =
-        RelocInfo::ModeMask(RelocInfo::WASM_PROTECTED_INSTRUCTION_LANDING);
-    for (RelocIterator it(code, mode_mask); !it.done(); it.next()) {
+    FixedArray* protected_instructions = code->protected_instructions();
+    DCHECK(protected_instructions != nullptr);
+    for (int i = 0; i < protected_instructions->length();
+         i += Code::kTrapDataSize) {
       trap_handler::ProtectedInstructionData data;
-      data.instr_offset = static_cast<uint32_t>(it.rinfo()->data());
-      data.landing_offset = static_cast<uint32_t>(it.rinfo()->pc() - base);
-      // Check that now over-/underflow happened.
-      DCHECK_EQ(it.rinfo()->data(), data.instr_offset);
-      DCHECK_EQ(it.rinfo()->pc() - base, data.landing_offset);
+      data.instr_offset =
+          protected_instructions
+              ->GetValueChecked<Smi>(isolate, i + Code::kTrapCodeOffset)
+              ->value();
+      data.landing_offset =
+          protected_instructions
+              ->GetValueChecked<Smi>(isolate, i + Code::kTrapLandingOffset)
+              ->value();
       unpacked.emplace_back(data);
     }
+
     if (unpacked.empty()) continue;
 
     const int index = RegisterHandlerData(base, code->instruction_size(),
                                           unpacked.size(), &unpacked[0]);
+
     unpacked.clear();
+
     // TODO(eholk): if index is negative, fail.
     DCHECK_LE(0, index);
     code->set_trap_handler_index(Smi::FromInt(index));

src/x64/assembler-x64.cc

@@ -4804,14 +4804,6 @@ void Assembler::emit_sse_operand(XMMRegister dst) {
   emit(0xD8 | dst.low_bits());
 }
 
-void Assembler::RecordProtectedInstructionLanding(int pc_offset) {
-  EnsureSpace ensure_space(this);
-  RelocInfo rinfo(pc(), RelocInfo::WASM_PROTECTED_INSTRUCTION_LANDING,
-                  pc_offset, nullptr);
-  reloc_info_writer.Write(&rinfo);
-}
-
-
 void Assembler::db(uint8_t data) {
   EnsureSpace ensure_space(this);
   emit(data);

src/x64/assembler-x64.h

@@ -1924,8 +1924,6 @@ class Assembler : public AssemblerBase {
     UNREACHABLE();
   }
 
-  void RecordProtectedInstructionLanding(int pc_offset);
-
   // Writes a single word of data in the code stream.
   // Used for inline tables, e.g., jump-tables.
   void db(uint8_t data);

test/cctest/compiler/test-code-generator.cc

@@ -764,7 +764,7 @@ class CodeGeneratorTester {
         frame_(environment->test_descriptor()->CalculateFixedFrameSize()),
         generator_(environment->main_zone(), &frame_, &linkage_,
                    environment->code(), &info_, base::Optional<OsrHelper>(),
-                   kNoSourcePosition, nullptr) {
+                   kNoSourcePosition, nullptr, nullptr) {
     // Force a frame to be created.
     generator_.frame_access_state()->MarkHasFrame(true);
     generator_.AssembleConstructFrame();