diff --git a/src/compiler/basic-block-instrumentor.cc b/src/compiler/basic-block-instrumentor.cc index 2d9b026dfa..86ec47979f 100644 --- a/src/compiler/basic-block-instrumentor.cc +++ b/src/compiler/basic-block-instrumentor.cc @@ -92,7 +92,8 @@ BasicBlockProfilerData* BasicBlockInstrumentor::Instrument( } else { counters_array = graph->NewNode(PointerConstant(&common, data->counts())); } - Node* one = graph->NewNode(common.Float64Constant(1)); + Node* zero = graph->NewNode(common.Int32Constant(0)); + Node* one = graph->NewNode(common.Int32Constant(1)); BasicBlockVector* blocks = schedule->rpo_order(); size_t block_number = 0; for (BasicBlockVector::iterator it = blocks->begin(); block_number < n_blocks; @@ -104,26 +105,37 @@ BasicBlockProfilerData* BasicBlockInstrumentor::Instrument( // It is unnecessary to wire effect and control deps for load and store // since this happens after scheduling. // Construct increment operation. - int offset_to_counter_value = static_cast(block_number) * kDoubleSize; + int offset_to_counter_value = static_cast(block_number) * kInt32Size; if (on_heap_counters) { offset_to_counter_value += ByteArray::kHeaderSize - kHeapObjectTag; } Node* offset_to_counter = graph->NewNode(IntPtrConstant(&common, offset_to_counter_value)); Node* load = - graph->NewNode(machine.Load(MachineType::Float64()), counters_array, + graph->NewNode(machine.Load(MachineType::Uint32()), counters_array, offset_to_counter, graph->start(), graph->start()); - Node* inc = graph->NewNode(machine.Float64Add(), load, one); - Node* store = graph->NewNode( - machine.Store(StoreRepresentation(MachineRepresentation::kFloat64, - kNoWriteBarrier)), - counters_array, offset_to_counter, inc, graph->start(), graph->start()); + Node* inc = graph->NewNode(machine.Int32Add(), load, one); + + // Branchless saturation, because we've already run the scheduler, so + // introducing extra control flow here would be surprising. + Node* overflow = graph->NewNode(machine.Uint32LessThan(), inc, load); + Node* overflow_mask = graph->NewNode(machine.Int32Sub(), zero, overflow); + Node* saturated_inc = + graph->NewNode(machine.Word32Or(), inc, overflow_mask); + + Node* store = + graph->NewNode(machine.Store(StoreRepresentation( + MachineRepresentation::kWord32, kNoWriteBarrier)), + counters_array, offset_to_counter, saturated_inc, + graph->start(), graph->start()); // Insert the new nodes. - static const int kArraySize = 6; - Node* to_insert[kArraySize] = {counters_array, one, offset_to_counter, - load, inc, store}; - // The first two Nodes are constant across all blocks. - int insertion_start = block_number == 0 ? 0 : 2; + static const int kArraySize = 10; + Node* to_insert[kArraySize] = { + counters_array, zero, one, offset_to_counter, + load, inc, overflow, overflow_mask, + saturated_inc, store}; + // The first three Nodes are constant across all blocks. + int insertion_start = block_number == 0 ? 0 : 3; NodeVector::iterator insertion_point = FindInsertionPoint(block); block->InsertNodes(insertion_point, &to_insert[insertion_start], &to_insert[kArraySize]); diff --git a/src/diagnostics/basic-block-profiler.cc b/src/diagnostics/basic-block-profiler.cc index eff80e6fca..20b6e567ea 100644 --- a/src/diagnostics/basic-block-profiler.cc +++ b/src/diagnostics/basic-block-profiler.cc @@ -60,7 +60,7 @@ Handle CopyStringToJSHeap(const std::string& source, Isolate* isolate) { } constexpr int kBlockIdSlotSize = kInt32Size; -constexpr int kBlockCountSlotSize = kDoubleSize; +constexpr int kBlockCountSlotSize = kInt32Size; } // namespace BasicBlockProfilerData::BasicBlockProfilerData( @@ -81,8 +81,7 @@ void BasicBlockProfilerData::CopyFromJSHeap( code_ = js_heap_data.code().ToCString().get(); ByteArray counts(js_heap_data.counts()); for (int i = 0; i < counts.length() / kBlockCountSlotSize; ++i) { - counts_.push_back( - reinterpret_cast(counts.GetDataStartAddress())[i]); + counts_.push_back(counts.get_uint32(i)); } ByteArray block_ids(js_heap_data.block_ids()); for (int i = 0; i < block_ids.length() / kBlockIdSlotSize; ++i) { @@ -112,7 +111,7 @@ Handle BasicBlockProfilerData::CopyToJSHeap( Handle counts = isolate->factory()->NewByteArray( counts_array_size_in_bytes, AllocationType::kOld); for (int i = 0; i < static_cast(n_blocks()); ++i) { - reinterpret_cast(counts->GetDataStartAddress())[i] = counts_[i]; + counts->set_uint32(i, counts_[i]); } Handle name = CopyStringToJSHeap(function_name_, isolate); Handle schedule = CopyStringToJSHeap(schedule_, isolate); @@ -133,7 +132,7 @@ void BasicBlockProfiler::ResetCounts(Isolate* isolate) { Handle counts( OnHeapBasicBlockProfilerData::cast(list->Get(i)).counts(), isolate); for (int j = 0; j < counts->length() / kBlockCountSlotSize; ++j) { - reinterpret_cast(counts->GetDataStartAddress())[j] = 0; + counts->set_uint32(j, 0); } } } @@ -197,9 +196,11 @@ void BasicBlockProfilerData::Log(Isolate* isolate) { } std::ostream& operator<<(std::ostream& os, const BasicBlockProfilerData& d) { - double block_count_sum = - std::accumulate(d.counts_.begin(), d.counts_.end(), 0); - if (block_count_sum == 0) return os; + if (std::all_of(d.counts_.cbegin(), d.counts_.cend(), + [](uint32_t count) { return count == 0; })) { + // No data was collected for this function. + return os; + } const char* name = "unknown function"; if (!d.function_name_.empty()) { name = d.function_name_.c_str(); @@ -210,14 +211,14 @@ std::ostream& operator<<(std::ostream& os, const BasicBlockProfilerData& d) { os << d.schedule_.c_str() << std::endl; } os << "block counts for " << name << ":" << std::endl; - std::vector> pairs; + std::vector> pairs; pairs.reserve(d.n_blocks()); for (size_t i = 0; i < d.n_blocks(); ++i) { pairs.push_back(std::make_pair(i, d.counts_[i])); } std::sort( pairs.begin(), pairs.end(), - [=](std::pair left, std::pair right) { + [=](std::pair left, std::pair right) { if (right.second == left.second) return left.first < right.first; return right.second < left.second; }); diff --git a/src/diagnostics/basic-block-profiler.h b/src/diagnostics/basic-block-profiler.h index 9753dcc3a1..edf6df0983 100644 --- a/src/diagnostics/basic-block-profiler.h +++ b/src/diagnostics/basic-block-profiler.h @@ -36,7 +36,7 @@ class BasicBlockProfilerData { DCHECK_EQ(block_ids_.size(), counts_.size()); return block_ids_.size(); } - const double* counts() const { return &counts_[0]; } + const uint32_t* counts() const { return &counts_[0]; } void SetCode(const std::ostringstream& os); void SetFunctionName(std::unique_ptr name); @@ -62,7 +62,7 @@ class BasicBlockProfilerData { // These vectors are indexed by reverse post-order block number. std::vector block_ids_; - std::vector counts_; + std::vector counts_; std::string function_name_; std::string schedule_; std::string code_; diff --git a/src/logging/log.cc b/src/logging/log.cc index a5a3ae2ac6..bd24014fd3 100644 --- a/src/logging/log.cc +++ b/src/logging/log.cc @@ -1088,7 +1088,7 @@ void Logger::TimerEvent(Logger::StartEnd se, const char* name) { } void Logger::BasicBlockCounterEvent(const char* name, int block_id, - double count) { + uint32_t count) { if (!FLAG_turbo_profiling_log_builtins) return; MSG_BUILDER(); msg << ProfileDataFromFileConstants::kBlockCounterMarker << kNext << name diff --git a/src/logging/log.h b/src/logging/log.h index bf929253f5..bf9fbfc7af 100644 --- a/src/logging/log.h +++ b/src/logging/log.h @@ -243,7 +243,7 @@ class Logger : public CodeEventListener { V8_EXPORT_PRIVATE void TimerEvent(StartEnd se, const char* name); - void BasicBlockCounterEvent(const char* name, int block_id, double count); + void BasicBlockCounterEvent(const char* name, int block_id, uint32_t count); void BuiltinHashEvent(const char* name, int hash); diff --git a/src/objects/shared-function-info.tq b/src/objects/shared-function-info.tq index 79bcd2a877..7d91c3ea02 100644 --- a/src/objects/shared-function-info.tq +++ b/src/objects/shared-function-info.tq @@ -97,7 +97,7 @@ class UncompiledDataWithPreparseData extends UncompiledData { @export class OnHeapBasicBlockProfilerData extends HeapObject { block_ids: ByteArray; // Stored as 4-byte ints - counts: ByteArray; // Stored as 8-byte floats + counts: ByteArray; // Stored as 4-byte unsigned ints name: String; schedule: String; code: String; diff --git a/test/cctest/compiler/test-basic-block-profiler.cc b/test/cctest/compiler/test-basic-block-profiler.cc index 795c1f1bd2..561f9a68b7 100644 --- a/test/cctest/compiler/test-basic-block-profiler.cc +++ b/test/cctest/compiler/test-basic-block-profiler.cc @@ -28,9 +28,21 @@ class BasicBlockProfilerTest : public RawMachineAssemblerTester { CHECK_NE(0, static_cast(l->size())); const BasicBlockProfilerData* data = l->back().get(); CHECK_EQ(static_cast(size), static_cast(data->n_blocks())); - const double* counts = data->counts(); + const uint32_t* counts = data->counts(); for (size_t i = 0; i < size; ++i) { - CHECK_EQ(static_cast(expected[i]), counts[i]); + CHECK_EQ(expected[i], counts[i]); + } + } + + void SetCounts(size_t size, uint32_t* new_counts) { + const BasicBlockProfiler::DataList* l = + BasicBlockProfiler::Get()->data_list(); + CHECK_NE(0, static_cast(l->size())); + BasicBlockProfilerData* data = l->back().get(); + CHECK_EQ(static_cast(size), static_cast(data->n_blocks())); + uint32_t* counts = const_cast(data->counts()); + for (size_t i = 0; i < size; ++i) { + counts[i] = new_counts[i]; } } }; @@ -73,6 +85,21 @@ TEST(ProfileDiamond) { uint32_t expected[] = {2, 1, 1, 1, 1, 2}; m.Expect(arraysize(expected), expected); } + + // Set the counters very high, to verify that they saturate rather than + // overflowing. + uint32_t near_overflow[] = {UINT32_MAX - 1, UINT32_MAX - 1, UINT32_MAX - 1, + UINT32_MAX - 1, UINT32_MAX - 1, UINT32_MAX - 1}; + m.SetCounts(arraysize(near_overflow), near_overflow); + m.Expect(arraysize(near_overflow), near_overflow); + + m.Call(0); + m.Call(0); + { + uint32_t expected[] = {UINT32_MAX, UINT32_MAX, UINT32_MAX, + UINT32_MAX - 1, UINT32_MAX - 1, UINT32_MAX}; + m.Expect(arraysize(expected), expected); + } }