cxxrtl: make observer methods non-virtual. (breaking change)

This avoids having to devirtualize them later to get performance back,
and simplifies the code a bit.

The change is prompted by the desire to add a similar observer object
to `eval()`, and a repeated consideration of whether the dispatch on
these should be virtual in first place.
This commit is contained in:
Catherine 2024-01-16 09:53:43 +00:00
parent ed81cc5f81
commit bf1a99da09
3 changed files with 9 additions and 19 deletions

View File

@ -2391,7 +2391,7 @@ struct CxxrtlWorker {
f << indent << "}\n"; f << indent << "}\n";
f << "\n"; f << "\n";
f << indent << "bool commit() override {\n"; f << indent << "bool commit() override {\n";
f << indent << indent << "null_observer observer;\n"; f << indent << indent << "observer observer;\n";
f << indent << indent << "return commit<>(observer);\n"; f << indent << indent << "return commit<>(observer);\n";
f << indent << "}\n"; f << indent << "}\n";
if (debug_info) { if (debug_info) {
@ -2486,7 +2486,7 @@ struct CxxrtlWorker {
f << indent << "}\n"; f << indent << "}\n";
f << "\n"; f << "\n";
f << indent << "bool commit() override {\n"; f << indent << "bool commit() override {\n";
f << indent << indent << "null_observer observer;\n"; f << indent << indent << "observer observer;\n";
f << indent << indent << "return commit<>(observer);\n"; f << indent << indent << "return commit<>(observer);\n";
f << indent << "}\n"; f << indent << "}\n";
if (debug_info) { if (debug_info) {

View File

@ -865,19 +865,12 @@ struct observer {
// Called when the `commit()` method for a wire is about to update the `chunks` chunks at `base` with `chunks` chunks // Called when the `commit()` method for a wire is about to update the `chunks` chunks at `base` with `chunks` chunks
// at `value` that have a different bit pattern. It is guaranteed that `chunks` is equal to the wire chunk count and // at `value` that have a different bit pattern. It is guaranteed that `chunks` is equal to the wire chunk count and
// `base` points to the first chunk. // `base` points to the first chunk.
virtual void on_update(size_t chunks, const chunk_t *base, const chunk_t *value) = 0; void on_update(size_t chunks, const chunk_t *base, const chunk_t *value) {}
// Called when the `commit()` method for a memory is about to update the `chunks` chunks at `&base[chunks * index]` // Called when the `commit()` method for a memory is about to update the `chunks` chunks at `&base[chunks * index]`
// with `chunks` chunks at `value` that have a different bit pattern. It is guaranteed that `chunks` is equal to // with `chunks` chunks at `value` that have a different bit pattern. It is guaranteed that `chunks` is equal to
// the memory element chunk count and `base` points to the first chunk of the first element of the memory. // the memory element chunk count and `base` points to the first chunk of the first element of the memory.
virtual void on_update(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) = 0; void on_update(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) {}
};
// The `null_observer` class has the same interface as `observer`, but has no invocation overhead, since its methods
// are final and have no implementation. This allows the observer feature to be zero-cost when not in use.
struct null_observer final: observer {
void on_update(size_t chunks, const chunk_t *base, const chunk_t *value) override {}
void on_update(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) override {}
}; };
template<size_t Bits> template<size_t Bits>
@ -916,8 +909,7 @@ struct wire {
// This method intentionally takes a mandatory argument (to make it more difficult to misuse in // This method intentionally takes a mandatory argument (to make it more difficult to misuse in
// black box implementations, leading to missed observer events). It is generic over its argument // black box implementations, leading to missed observer events). It is generic over its argument
// to make sure the `on_update` call is devirtualized. This is somewhat awkward but lets us keep // to allow the `on_update` method to be non-virtual.
// a single implementation for both this method and the one in `memory`.
template<class ObserverT> template<class ObserverT>
bool commit(ObserverT &observer) { bool commit(ObserverT &observer) {
if (curr != next) { if (curr != next) {

View File

@ -556,22 +556,20 @@ public:
bool record_incremental(ModuleT &module) { bool record_incremental(ModuleT &module) {
assert(streaming); assert(streaming);
struct : public observer { struct {
std::unordered_map<const chunk_t*, spool::ident_t> *ident_lookup; std::unordered_map<const chunk_t*, spool::ident_t> *ident_lookup;
spool::writer *writer; spool::writer *writer;
CXXRTL_ALWAYS_INLINE CXXRTL_ALWAYS_INLINE
void on_update(size_t chunks, const chunk_t *base, const chunk_t *value) override { void on_update(size_t chunks, const chunk_t *base, const chunk_t *value) {
writer->write_change(ident_lookup->at(base), chunks, value); writer->write_change(ident_lookup->at(base), chunks, value);
} }
CXXRTL_ALWAYS_INLINE CXXRTL_ALWAYS_INLINE
void on_update(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) override { void on_update(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) {
writer->write_change(ident_lookup->at(base), chunks, value, index); writer->write_change(ident_lookup->at(base), chunks, value, index);
} }
} record_observer; } record_observer = { &ident_lookup, &writer };
record_observer.ident_lookup = &ident_lookup;
record_observer.writer = &writer;
writer.write_sample(/*incremental=*/true, pointer++, timestamp); writer.write_sample(/*incremental=*/true, pointer++, timestamp);
for (auto input_index : inputs) { for (auto input_index : inputs) {