Skip to content

sc_fifo<T> should use placement new #124

@jnbrq

Description

@jnbrq

Consider that we have an sc_fifo<sc_bv_base> to store bitvectors of unknown widths:

struct Module : public sc_module {
    Module(sc_module_name const& name = "")
        : sc_module(name)
        , SC_NAMED(fifo) {
        SC_THREAD(produce);
        SC_THREAD(consume);
    }

private:
    sc_fifo<sc_bv_base> fifo;

    void print(sc_bv_base const& bv) {
        std::cout << "length: " << bv.length() << ", value: " << std::hex << "0x" << bv.to_uint64() << std::dec << "\n";
    }

    void produce() {
        fifo.write(sc_bv<32>(0xDEAD'BEEF));
        fifo.write(sc_bv<64>(0xDEAD'BEEF'DEAD'BEEFull));
        fifo.write(sc_bv_base(48));
        fifo.write(sc_bv_base(16));
    }

    void consume() {
        print(fifo.read());
        print(fifo.read());
        print(fifo.read());
        print(fifo.read());
    }
};

This code produces the following output:


        SystemC 3.0.0-Accellera --- Jun 21 2024 11:48:28
        Copyright (c) 1996-2024 by all Contributors,
        ALL RIGHTS RESERVED
length: 32, value: 0xdeadbeef
length: 32, value: 0xdeadbeef
length: 32, value: 0x0
length: 32, value: 0x0

I do not think this is expected, I would expect that when a new entry is pushed to the FIFO, the output entry should have the same bit width as the pushed entry.

The reason is that the current implementation of sc_fifo uses an array of default-initialized Ts for storage, and sc_bv_base has an assignment operator that does not inherit the bit width of the RHS operand. Default-initialized sc_bv_base has a bit width of 32.

I think the behavior of sc_bv_base::operator= is expected; however, sc_fifo<T> behaves incorrectly. sc_fifo<T> implementation should closely mimic the std::vector<T> implementation that uses (1) uninitialized storage, (2) placement new operator, and (3) explicit destruction. So, at least the following changes should be made:

template <class T>
inline
bool
sc_fifo<T>::buf_write( const T& val_ )
{
    if( m_free == 0 ) {
	return false;
    }
    // m_buf[m_wi] = val_;
    new (&m_buf[m_wi]) T(val_); // changed
    m_wi = ( m_wi + 1 ) % m_size;
    m_free --;
    return true;
}

template <class T>
inline
bool
sc_fifo<T>::buf_read( T& val_ )
{
    if( m_free == m_size ) {
	return false;
    }
    // val_ = m_buf[m_ri];
    new (&val_) T(m_buf[m_ri]); // changed
    // m_buf[m_ri] = T(); // clear entry for boost::shared_ptr, et al.
    m_buf[m_ri].~T(); // changed
    m_ri = ( m_ri + 1 ) % m_size;
    m_free ++;
    return true;
}

I managed to reproduce this bug on Linux and Mac, x86_64 and arm64 architectures, using the latest SystemC release.
You can clone the following repository, which contains a minimal code example: https://github.com/jnbrq/sysc_fifo_bug

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions