Update according review comments

This commit is contained in:
Yu-Sheng Lin 2026-03-23 02:02:24 +08:00
parent 76f241c226
commit 04e4f7b63d
6 changed files with 32 additions and 44 deletions

View File

@ -64,7 +64,7 @@ In brief, to install from git:
#sudo apt-get install libgoogle-perftools-dev libjemalloc-dev numactl perl-doc
#sudo apt-get install libfl2 # Ubuntu only (ignore if gives error)
#sudo apt-get install libfl-dev # Ubuntu only (ignore if gives error)
#sudo apt-get install zlibc zlib1g zlib1g-dev # Ubuntu only (ignore if gives error)
#sudo apt-get install zlibc zlib1g zlib1g-dev liblz4 liblz4-dev # Ubuntu only (ignore if gives error)
git clone https://github.com/verilator/verilator # Only first time
@ -116,7 +116,7 @@ To build or run Verilator, you need these standard packages:
sudo apt-get install libgz # Non-Ubuntu (ignore if gives error)
sudo apt-get install libfl2 # Ubuntu only (ignore if gives error)
sudo apt-get install libfl-dev # Ubuntu only (ignore if gives error)
sudo apt-get install zlibc zlib1g zlib1g-dev # Ubuntu only (ignore if gives error)
sudo apt-get install zlibc zlib1g zlib1g-dev liblz4 liblz4-dev # Ubuntu only (ignore if gives error)
For SystemC:

View File

@ -23,13 +23,13 @@
#include "fstcpp/fstcpp_stream_write_helper.h"
#include "fstcpp/fstcpp_variable_info.h"
// AT(x) is used to access vector at index x, and it will throw exception if out of bound
// AT(vec, x) is used to access vector at index x, and it will throw exception if out of bound
// in debug mode, but in release mode, it will not throw exception
// Usually you should only need AT(x) only at very hot code path.
// Usually you should only need AT(vec, x) only at very hot code path.
#ifndef NDEBUG
# define AT(x) .at(x)
# define AT(vec, x) (vec.at(x))
#else
# define AT(x) [x]
# define AT(vec, x) (vec[x])
#endif
namespace fst {
@ -228,7 +228,7 @@ void Writer::emitTimeChange(uint64_t tim) {
template <typename... T>
void Writer::emitValueChangeHelper_(Handle handle, T &&...val) {
// Let data prefetch go first
VariableInfo &var_info = m_value_change_data_.m_variable_infos AT(handle - 1);
VariableInfo &var_info = AT(m_value_change_data_.m_variable_infos, handle - 1);
#if defined(__GNUC__) || defined(__clang__)
__builtin_prefetch(var_info.data_ptr() + var_info.size() - 1, 1, 0);
#endif
@ -256,7 +256,7 @@ void Writer::emitValueChange(Handle handle, uint64_t val) {
void Writer::emitValueChange(Handle handle, const char *val) {
finalizeHierarchy_();
VariableInfo &var_info = m_value_change_data_.m_variable_infos AT(handle - 1);
VariableInfo &var_info = AT(m_value_change_data_.m_variable_infos, handle - 1);
// For double handles, const char* is interpreted as a double* (8B)
// This double shall be written out as raw IEEE 754 double
@ -271,24 +271,23 @@ void Writer::emitValueChange(Handle handle, const char *val) {
FST_DCHECK_NE(bitwidth, 0);
val += bitwidth;
static std::vector<uint64_t> t_packed_value_buffer;
const unsigned num_words{(bitwidth + 63) / 64};
t_packed_value_buffer.assign(num_words, 0);
m_packed_value_buffer_.assign(num_words, 0);
for (unsigned i = 0; i < num_words; ++i) {
const char *start{val - std::min((i + 1) * 64, bitwidth)};
const char *end{val - 64 * i};
t_packed_value_buffer[i] = 0;
m_packed_value_buffer_[i] = 0;
for (const char *p = start; p < end; ++p) {
// No checking for invalid characters, follow original C implementation
t_packed_value_buffer[i] <<= 1;
t_packed_value_buffer[i] |= static_cast<uint64_t>(*p - '0');
m_packed_value_buffer_[i] <<= 1;
m_packed_value_buffer_[i] |= static_cast<uint64_t>(*p - '0');
}
}
if (bitwidth <= 64) {
emitValueChange(handle, t_packed_value_buffer.front());
emitValueChange(handle, m_packed_value_buffer_.front());
} else {
emitValueChange(handle, t_packed_value_buffer.data(), EncodingType::BINARY);
emitValueChange(handle, m_packed_value_buffer_.data(), EncodingType::BINARY);
}
}

View File

@ -77,6 +77,9 @@ private:
std::ofstream m_main_fst_file_{};
std::vector<uint8_t> m_hierarchy_buffer_{};
std::vector<uint8_t> m_geometry_buffer_{};
// Temporary buffer for packing bit strings into words
// Only used in emitValueChange(Handle, const char*)
std::vector<uint64_t> m_packed_value_buffer_{};
Header m_header_{};
detail::BlackoutData m_blackout_data_{}; // Not implemented yet
detail::ValueChangeData m_value_change_data_{};

View File

@ -104,7 +104,6 @@ CPPFLAGS += -I. $(VK_CPPFLAGS_WALL) $(VK_CPPFLAGS_ALWAYS)
VPATH += ..
VPATH += $(VERILATOR_ROOT)/include
VPATH += $(VERILATOR_ROOT)/include/vltstd
VPATH += $(VERILATOR_ROOT)/include/fstcpp
LDFLAGS += $(CFG_LDFLAGS_VERILATED)

View File

@ -395,61 +395,48 @@ VL_ATTR_ALWINLINE
void VerilatedFstBuffer::emitBit(uint32_t code, CData newval) {
VL_DEBUG_IFDEF(assert(m_symbolp[code]););
m_owner.emitTimeChangeMaybe();
m_fst->emitValueChange(m_symbolp[code], newval ? 1 : 0);
m_fst->emitValueChange(m_symbolp[code], uint64_t(newval));
}
VL_ATTR_ALWINLINE
void VerilatedFstBuffer::emitCData(uint32_t code, CData newval, int bits) {
void VerilatedFstBuffer::emitCData(uint32_t code, CData newval, int) {
VL_DEBUG_IFDEF(assert(m_symbolp[code]););
m_owner.emitTimeChangeMaybe();
m_fst->emitValueChange(m_symbolp[code], newval);
}
VL_ATTR_ALWINLINE
void VerilatedFstBuffer::emitSData(uint32_t code, SData newval, int bits) {
void VerilatedFstBuffer::emitSData(uint32_t code, SData newval, int) {
VL_DEBUG_IFDEF(assert(m_symbolp[code]););
m_owner.emitTimeChangeMaybe();
m_fst->emitValueChange(m_symbolp[code], newval);
}
VL_ATTR_ALWINLINE
void VerilatedFstBuffer::emitIData(uint32_t code, IData newval, int bits) {
void VerilatedFstBuffer::emitIData(uint32_t code, IData newval, int) {
VL_DEBUG_IFDEF(assert(m_symbolp[code]););
m_owner.emitTimeChangeMaybe();
m_fst->emitValueChange(m_symbolp[code], newval);
}
VL_ATTR_ALWINLINE
void VerilatedFstBuffer::emitQData(uint32_t code, QData newval, int bits) {
void VerilatedFstBuffer::emitQData(uint32_t code, QData newval, int) {
VL_DEBUG_IFDEF(assert(m_symbolp[code]););
m_owner.emitTimeChangeMaybe();
m_fst->emitValueChange(m_symbolp[code], newval);
}
VL_ATTR_ALWINLINE
void VerilatedFstBuffer::emitWData(uint32_t code, const WData* newvalp, int bits) {
// While emitValueChange has a uint32_t* version
// It does the same conversion, allocating a pointer and copying the data
// So I decide to use the verilator buffer directly.
// The buffer were designed to hold maxBits() char,
// so it is very safe to use it as uint64_t*.
int words = VL_WORDS_I(bits);
uint64_t* wp = reinterpret_cast<uint64_t*>(m_strbufp);
// cast newvalp (uint32_t[words]) to wp (uint64_t[ceil(words/2)])
for (int i = 0; i < words/2; ++i) {
wp[i] = newvalp[i*2+1];
wp[i] <<= 32;
wp[i] |= newvalp[i*2];
}
if (words % 2 == 1) {
wp[words/2] = newvalp[words-1];
}
void VerilatedFstBuffer::emitWData(uint32_t code, const WData* newvalp, int) {
VL_DEBUG_IFDEF(assert(m_symbolp[code]););
m_owner.emitTimeChangeMaybe();
m_fst->emitValueChange(m_symbolp[code], wp);
// call emitValueChange(handle, uint32_t*)
m_fst->emitValueChange(m_symbolp[code], newvalp);
}
VL_ATTR_ALWINLINE
void VerilatedFstBuffer::emitDouble(uint32_t code, double newval) {
VL_DEBUG_IFDEF(assert(m_symbolp[code]););
m_owner.emitTimeChangeMaybe();
uint64_t newval_u64;
std::memcpy(&newval_u64, &newval, sizeof(newval_u64));

View File

@ -231,11 +231,11 @@ class VerilatedFstBuffer VL_NOT_FINAL {
// called from only one place (the full* methods), so always inline them.
VL_ATTR_ALWINLINE void emitEvent(uint32_t code);
VL_ATTR_ALWINLINE void emitBit(uint32_t code, CData newval);
VL_ATTR_ALWINLINE void emitCData(uint32_t code, CData newval, int bits);
VL_ATTR_ALWINLINE void emitSData(uint32_t code, SData newval, int bits);
VL_ATTR_ALWINLINE void emitIData(uint32_t code, IData newval, int bits);
VL_ATTR_ALWINLINE void emitQData(uint32_t code, QData newval, int bits);
VL_ATTR_ALWINLINE void emitWData(uint32_t code, const WData* newvalp, int bits);
VL_ATTR_ALWINLINE void emitCData(uint32_t code, CData newval, int);
VL_ATTR_ALWINLINE void emitSData(uint32_t code, SData newval, int);
VL_ATTR_ALWINLINE void emitIData(uint32_t code, IData newval, int);
VL_ATTR_ALWINLINE void emitQData(uint32_t code, QData newval, int);
VL_ATTR_ALWINLINE void emitWData(uint32_t code, const WData* newvalp, int);
VL_ATTR_ALWINLINE void emitDouble(uint32_t code, double newval);
};