Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with update to Version 1.5.15 from 1.5.9 #62

Closed
Milemarco opened this issue Oct 9, 2023 · 7 comments
Closed

Problems with update to Version 1.5.15 from 1.5.9 #62

Milemarco opened this issue Oct 9, 2023 · 7 comments

Comments

@Milemarco
Copy link
Contributor

Hi there,

I recently attempted to integrate my project with a newer version of ICSC and encountered an error related to records using non-default constructors. You can check out the error
image

The problematic code snippet is as follows:

// MArbiter Selector input/output type...
template<int DWIDTH = 1, int SEL_MAX = 1>
struct SSelectorIO {
    sc_uint<DWIDTH> dat;
    sc_uint<NUM_BITS(SEL_MAX)> sel;
    bool sel_valid;

    SSelectorIO(sc_uint<DWIDTH> _dat = 0, sc_uint<NUM_BITS(SEL_MAX)> _sel = 0, bool _sel_valid = 0):
        dat(_dat), sel(_sel), sel_valid(_sel_valid) { }

    // Necessary operators for using this structure as signals...
    // Compare (necessary for signal updates)...
    bool operator == (const SSelectorIO &t) {
        return (dat == t.dat) && (sel == t.sel) && (sel_valid == t.sel_valid);
    }
    // Display...
    friend ostream& operator<< (ostream &o, const SSelectorIO &t ) {
        o << "{" << t.dat << "," << t.sel << "," << t.sel_valid << "}" ;
        return o;
    }
    // Tracing...
    friend void sc_trace( sc_trace_file *tf, const SSelectorIO &t, const std::string &name ) {
        PN_TRACE_R (tf, t, dat, name);
        PN_TRACE_R (tf, t, sel, name);
        PN_TRACE_R (tf, t, sel_valid, name);
    }
};

I'd like to know if this behavior is intentional and if there are plans to reintroduce support for it in a future update.

Best regards,

  • Marco
@mikhailmoiseev
Copy link
Contributor

Hi Marco,

Generally, for record type used in array there is no initializer list and no constructor body generated. So, only default constructor behavior is supported.

In your case you could add default constructor SSelectorIO() = default.
Do you see any cases when that does not work or any problems?

-Mikhail.

@Milemarco
Copy link
Contributor Author

Hi Mikhail,

Your suggestion worked for me.

I changed the struct to this:

// MArbiter Selector input/output type...
template<int DWIDTH = 1, int SEL_MAX = 1>
struct SSelectorIO {
    sc_uint<DWIDTH> dat;
    sc_uint<NUM_BITS(SEL_MAX)> sel;
    bool sel_valid;

    // SSelectorIO( sc_uint<DWIDTH> _dat = 0,  sc_uint<NUM_BITS(SEL_MAX)> _sel = 0, bool _sel_valid = 0): 
    //     dat(_dat), sel(_sel), sel_valid(_sel_valid) { }
    SSelectorIO() = default;
	
    void initSelector(sc_uint<DWIDTH> data, sc_uint<NUM_BITS(SEL_MAX)> selBits, bool sel_valid) {
        dat = data;
        sel = selBits;
        sel_valid = sel_valid;
    }


    // Necessary operators for using this structure as signals...
    // Compare (necessary for signal updates)...
    bool operator == (const SSelectorIO &t) {
       return (dat == t.dat) && (sel == t.sel) && (sel_valid == t.sel_valid);
    }
    // Display...
    friend ostream& operator<< (ostream &o, const SSelectorIO &t ) {
       o << "{" << t.dat << "," << t.sel << "," << t.sel_valid << "}" ;
       return o;
    }
    // Tracing...
    friend void sc_trace( sc_trace_file *tf, const SSelectorIO &t, const std::string &name ) {
        PN_TRACE_R (tf, t, dat, name);
        PN_TRACE_R (tf, t, sel, name);
        PN_TRACE_R (tf, t, sel_valid, name);
    }
 };

Synthesis worked again with this changes. Hardware tests are still to be done.

Thank you for your help here.

I noticed another strange behaviour when translating to verilog.

This Code results in a weird SV translation

uint n_max = CFG_NUT_CPU_CORES/CFG_MEMU_BANK_RAM_PORTS;
            if(CFG_MEMU_BANK_RAM_PORTS > 1 && p == 0)
                n_max = n_max-1; // This produces (signed) in the equation

results in SV:

n_max = 2'(1 <<< 1) / 2;
            if (2 > 1 && signed'({1'b0, p_2}) == 0)
            begin
                n_max = signed'({1'b0, n_max}) - 1;
            end

It shouldn't be casted to a signed here. I have to rewrite it to n_max = n_max-1U then the problem is solved.

Best regards

  • Marco

@mikhailmoiseev
Copy link
Contributor

n_max = n_max-1 can be potentially negative and as soon as 1 is signed int, first argument n_max converted to signed.
There is an option for design where only unsigned arithmetics is used UNSIGNED see details here: https://github.com/intel/systemc-compiler/wiki/Tool-options-and-defines

I do not like how if condition in your example is translated. Will check that. What is the type of CFG_MEMU_BANK_RAM_PORTS?

@Milemarco
Copy link
Contributor Author

The 'CFG_MEMU_BANK_RAM_PORTS' is defined like this #define CFG_MEMU_BANK_RAM_PORTS 2U

@Milemarco
Copy link
Contributor Author

I just noticed an other maybe related Issue with signed unsigned conversion.
We use this macro to determine number of Cores:

#define CFG_NUT_CPU_CORES_LD 1U
#define CFG_NUT_CPU_CORES (1U << CFG_NUT_CPU_CORES_LD)
#define CFG_MEMU_BANK_RAM_PORTS 2U

in code we use it like this:
uint8_t n_max = CFG_NUT_CPU_CORES/CFG_MEMU_BANK_RAM_PORTS;
The icsc->sv->v synthesis translates it to this:
n_max = 2'sd2 / 2;
I even tried casting it to a uint like this
uint8_t n_max = ((uint8_t)CFG_NUT_CPU_CORES)/CFG_MEMU_BANK_RAM_PORTS;
which results in
n_max = 8'sd2 / 2;
The only way I find to force it to be an unsigned is by usign a variable like so:

uint8_t temp_helper = (CFG_NUT_CPU_CORES);
uint8_t n_max = ((uint8_t)temp_helper)/CFG_MEMU_BANK_RAM_PORTS;

@mikhailmoiseev
Copy link
Contributor

OK, will check that.

@mikhailmoiseev
Copy link
Contributor

In the last version 1.6 with updated SC there is no issues, generated code:

n_max = 2'(1 <<< 1) / 2;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants