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

error: ScTool internal error : Record owner is not variable #80

Open
libalis opened this issue Feb 9, 2025 · 35 comments
Open

error: ScTool internal error : Record owner is not variable #80

libalis opened this issue Feb 9, 2025 · 35 comments

Comments

@libalis
Copy link

libalis commented Feb 9, 2025

First of all, I want to thank you very much for this great project and all the hard work that has gone into it.
I am currently pursuing my Bachelor's degree in Computer Science and it's been incredibly helpful for my studies.
Since this is my first time using SystemC, I apologize in advance for any poorly written code.

Description:

When running the following command:

ctest -R sata --rerun-failed --output-on-failure

I encountered this error:

--------------------------------------------------------------
Intel Compiler for SystemC (ICSC) version 1.6.10, Dec 06, 2024
--------------------------------------------------------------
Top module is MATRIX_VECTOR
Elaboration database created

error: ScTool internal error : Record owner is not variable
sata_sctool: /home/robert/Downloads/sc_tools/icsc/sc_tool/lib/sc_tool/cfg/SValue.h:428: sc::SRecord::SRecord(const clang::QualType&, std::vector<sc::SValue>, const sc::SValue&, const sc::SValue&, size_t): Assertion `false' failed.

The issue occurs during the synthesis process.
You can find my repository here: https://github.com/libalis/bachelor.

Additional Information:

  • Intel Compiler for SystemC (ICSC): Version 1.6.10 (Dec 06, 2024)
  • Top module: MATRIX_VECTOR

I would greatly appreciate any help or insights regarding this issue.
Thank you again for all the effort you’ve put into this amazing project!

@mikhailmoiseev
Copy link
Contributor

In the project, there is btint class which has user-defined default constructor btint(). Such classes cannot be used in arrays as soon as the default constructor code should be substituted at the array declaration, which makes the generated code ugly and unreadable. Have updated the compiler to print a reasonable error message.

Generally, I would recommend to rid of btint and replace it with sc_biguint<2*T+1> with correspondent functions. The class implementation has lots of extra things like function calls which will be resulted in extra logic. Classes which are not modules have limited usage in synthesizable SystemC.

@libalis
Copy link
Author

libalis commented Feb 11, 2025

Thank you very much for the quick reply and the time you spent.
Are custom data types generally not supported by the compiler, or only in arrays, or only those with a standard constructor?
My implementation is based on this: https://www.learnsystemc.com/basic/customized_datatype
Are helper functions like the ones I use capable of synthesis?

@mikhailmoiseev
Copy link
Contributor

There is synthesizable subset of SystemC which is supported for hardware synthesis.
For ICSC this subset explained at here.

Custom data types are generally supported. Helper functions are also supported in most cases.

@libalis
Copy link
Author

libalis commented Feb 11, 2025

Thank you for the resources, they are very helpful.
Would removing the standard constructor solve the problem or should I really get rid of the custom data type and just leave the helper functions?

@mikhailmoiseev
Copy link
Contributor

That needs to be checked. Normally you run a compiler periodically for a new portion of code to ensure the code syntaxis is correct. As soon as you come with large amount of code it is difficult to say how many fixes it requires.

General recommendation is to think of which HW cells are generated for SystemC statements you are writing. Many places can be simplified and rewritten more efficiently from HW viewpoint.

@libalis
Copy link
Author

libalis commented Feb 11, 2025

I understand, I will definitely take that into account and do it that way, if necessary I will get in touch again.
Thank you for everything.

@libalis
Copy link
Author

libalis commented Feb 18, 2025

I am currently trying to synthesize each module one by one, now I got this error:

--------------------------------------------------------------
 Intel Compiler for SystemC (ICSC) version 1.6.12, Feb 10,2024
--------------------------------------------------------------
Top module is ADDER_SUBTRACTOR
Elaboration database created

error: ScTool internal error : SValue::getRecord() incorrect type
sata_sctool: /home/robert/Downloads/sc_tools/icsc/sc_tool/lib/sc_tool/cfg/SValue.cpp:480: sc::SRecord& sc::SValue::getRecord() const: Assertion `false' failed.

What does this error mean, how can I fix it?

@mikhailmoiseev
Copy link
Contributor

Could you provide updated Cmake with svc_target for this target? I see two instances of ADDER_SUBTRACTOR module.

@libalis
Copy link
Author

libalis commented Feb 19, 2025

I just added a new adder subtractor instance in the system module.
Please excuse the txt file extension, GItHub does not allow cpp uploads.
CMakeLists.txt
main.txt

@libalis
Copy link
Author

libalis commented Feb 19, 2025

Although I have just pushed a new commit, I am still referring to this:
https://github.com/libalis/bachelor/tree/273135990479ef35746a219bffe1d8dc79a33ca7

@mikhailmoiseev
Copy link
Contributor

That not compiled at my side:
matrix/Source Code/cpp/main.cpp:30:45: error: no match for call to ‘(sc_core::sc_in<sc_dt::sc_biguint<17> >) (sc_core::sc_signal<btint<8>, sc_core::SC_ONE_WRITER>&)’
2122: 30 | adder_subtractor->adder_subtractor_a(adder_subtractor_a);
2122: | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~

Please add 2nd compilation target in CMakeLIst.txt with another main.cpp or ifdef in the current one. Try that it is compiled and passed stage BUILD in ICSC at your side. I wont do any changes in your code.

@libalis
Copy link
Author

libalis commented Feb 20, 2025

Thank you for your patience.
As I mentioned, the error message was from an older commit.
I’ve created and attached a zip file with the updated main and CMakeLists.
It works on my end.
Thanks again for your help!
bachelor.zip

@mikhailmoiseev
Copy link
Contributor

The problem was in return *this; in btint set_value(int index, int decimal_value) -- dereference of this is not supported. When I replaced btint() with btint() = default, it synthesized.

For btint class I strongly recommend you to remove all the constructors, replace them with simple initialization functions. Instead of using set_value with individual bit setting, set whole values if possible.

Another thing, there is almost no reason to use any pointers. Instead of SC object pointers array it needs to use sc_vector. We do not have any pointers in industrial designs.

@libalis
Copy link
Author

libalis commented Feb 21, 2025

Thank you so much, that was so helpful!
I will definitely implement all of your suggestions.
Just one stupid question: do you have an example of how to create and use vectors?

@mikhailmoiseev
Copy link
Contributor

You could find examples in designs/examples and designs/tests folders. For sc_vector there is https://github.com/intel/systemc-compiler/blob/main/designs/tests/misc/test_sc_vector1.cpp and many others.

@libalis
Copy link
Author

libalis commented Feb 21, 2025

Thank you, but didn't you suggest an sc object vector?
How do you initialize one?
Please excuse me, as I'm new to this topic.

@mikhailmoiseev
Copy link
Contributor

SC objects are all the classes inherit sc_object, including modules, signals, ports and others.

@libalis
Copy link
Author

libalis commented Feb 21, 2025

Understood, but don't modules have constructors that need to be called?

@mikhailmoiseev
Copy link
Contributor

There is an option how to call module constructor with additional parameters. See IEEE1666 for more details.

sc_vector<sct_initiator<T>>    init{"init", N,         // Three parameters
      [](const char* name, size_t i) {                 // Lambda function         
           return sc_new<sct_initiator<T>>(name, 1);   // Initiator with sync register
      }}; 

@libalis
Copy link
Author

libalis commented Feb 21, 2025

I'll give it a try, thank you very much.

@libalis
Copy link
Author

libalis commented Feb 23, 2025

Thank you very much, the adder subtractor actually synthesizes!
Unfortunately, however, I have encountered new problems in the version I just pushed:

  1. Why is this a warning and not an error?
/home/robert/Downloads/sc_tools/designs/Bachelor/Source Code/cpp/../hpp/shift_register.hpp:12:9: warning: Channel read in CTHREAD reset section is prohibited : shift_register_state
   12 |         sc_out<btint<T>> shift_register_state;
      |         ^
  1. Unfortunately I don't understand this error message, what do I have to do to fix it?
/home/robert/Downloads/sc_tools/designs/Bachelor/Source Code/cpp/shift_register.cpp:9:36: error: No record object found for member call : sc_out<struct btint<8>> system.shift_register.port_3_REC_3 sc_out<struct btint<8>> system.shift_register.port_3_REC_3
    9 |         shift_register_state.write(shift_register_state.read().shift_right(1).set_value(T - 1, shift_register_input.read().get_value(0)));
      |                                    ^
  1. Why does range() work in the module but not in btint?
/home/robert/Downloads/sc_tools/designs/Bachelor/Source Code/cpp/../hpp/btint.hpp:86:44: fatal error: Incorrect range access, different variables in lo/hi
   86 |             output.btint_a = btint_a.range(from, to);
      |                                            ^

This works:

btint<T> output;
output.btint_a = shift_register_input.read().btint_a.range(T, 1);
output.btint_b = shift_register_input.read().btint_b.range(T, 1);
output = output.set_overflow(shift_register_input.read().get_overflow());
shift_register_output.write(output);

But not this:

shift_register_output.write(shift_register_input.read().range(T, 1));

Please excuse me for taking up so much of your time, I really appreciate it!

@mikhailmoiseev
Copy link
Contributor

  1. No channel can be read in reset section as soon as "it does not have value in reset", so that does not make sense.
  2. Too complicated expression. No method call for temporary variable returned by another method call supported. Split the expression into multiple.
  3. Range must have constant arguments or one variable argument and another based on it: range(var+C, var). That is same in Verilog.

@libalis
Copy link
Author

libalis commented Feb 24, 2025

It has successfully synthesized, thank you so much!
Just to clarify, would it have still synthesized if there were warnings present?
I’ll be working on the multiplier next.
Hopefully, with all your helpful tips, it should go smoothly!

@mikhailmoiseev
Copy link
Contributor

Generated RTL with warnings can be correct, but it is good practice to check the warnings.

@libalis
Copy link
Author

libalis commented Feb 24, 2025

Makes sense, thank you.

@libalis
Copy link
Author

libalis commented Mar 7, 2025

I've been trying for quite some time now to generate a bitstream from the successfully synthesized SystemVerilog file using Vivado.
I've already managed to fix several issues by making adjustments to the SystemC code.
However, I'm struggling with the following two errors and could really use your help:

[DRC NSTD-1] Unspecified I/O Standard: 130 out of 130 logical ports use I/O standard (IOSTANDARD) value 'DEFAULT', instead of a user assigned specific value. This may cause I/O contention or incompatibility with the board power or connectivity affecting performance, signal integrity or in extreme cases cause damage to the device or the components to which it is connected. To correct this violation, specify all I/O standards. This design will fail to generate a bitstream unless all logical ports have a user specified I/O standard value defined. To allow bitstream creation with unspecified I/O standard values (not recommended), use this command: set_property SEVERITY {Warning} [get_drc_checks NSTD-1]. NOTE: When using the Vivado Runs infrastructure (e.g. launch_runs Tcl command), add this command to a .tcl file and add that file as a pre-hook for write_bitstream step for the implementation run. Problem ports: matrix_vector_matrix_btint_a[0][0][3:0], matrix_vector_matrix_btint_a[0][1][3:0], matrix_vector_matrix_btint_a[0][2][3:0], matrix_vector_matrix_btint_a[1][0][3:0], matrix_vector_matrix_btint_a[1][1][3:0], matrix_vector_matrix_btint_a[1][2][3:0], matrix_vector_matrix_btint_a[2][0][3:0], matrix_vector_matrix_btint_a[2][1][3:0], matrix_vector_matrix_btint_a[2][2][3:0], matrix_vector_matrix_btint_b[0][0][3:0], matrix_vector_matrix_btint_b[0][1][3:0], matrix_vector_matrix_btint_b[0][2][3:0], matrix_vector_matrix_btint_b[1][0][3:0], matrix_vector_matrix_btint_b[1][1][3:0], matrix_vector_matrix_btint_b[1][2][3:0]... and (the first 15 of 37 listed).

[DRC UCIO-1] Unconstrained Logical Port: 130 out of 130 logical ports have no user assigned specific location constraint (LOC). This may cause I/O contention or incompatibility with the board power or connectivity affecting performance, signal integrity or in extreme cases cause damage to the device or the components to which it is connected. To correct this violation, specify all pin locations. This design will fail to generate a bitstream unless all logical ports have a user specified site LOC constraint defined. To allow bitstream creation with unspecified pin locations (not recommended), use this command: set_property SEVERITY {Warning} [get_drc_checks UCIO-1]. NOTE: When using the Vivado Runs infrastructure (e.g. launch_runs Tcl command), add this command to a .tcl file and add that file as a pre-hook for write_bitstream step for the implementation run. Problem ports: matrix_vector_matrix_btint_a[0][0][3:0], matrix_vector_matrix_btint_a[0][1][3:0], matrix_vector_matrix_btint_a[0][2][3:0], matrix_vector_matrix_btint_a[1][0][3:0], matrix_vector_matrix_btint_a[1][1][3:0], matrix_vector_matrix_btint_a[1][2][3:0], matrix_vector_matrix_btint_a[2][0][3:0], matrix_vector_matrix_btint_a[2][1][3:0], matrix_vector_matrix_btint_a[2][2][3:0], matrix_vector_matrix_btint_b[0][0][3:0], matrix_vector_matrix_btint_b[0][1][3:0], matrix_vector_matrix_btint_b[0][2][3:0], matrix_vector_matrix_btint_b[1][0][3:0], matrix_vector_matrix_btint_b[1][1][3:0], matrix_vector_matrix_btint_b[1][2][3:0]... and (the first 15 of 37 listed).

Unfortunately, I’m not familiar with SystemVerilog, which makes it difficult for me to understand these messages - let alone fix them.
Would it be possible to resolve these errors within the SystemC code as well?
If so, I would be incredibly grateful for any suggestions.
Alternatively, could the compiler be adjusted to automatically generate the necessary modifications?
I truly appreciate your help - it's only thanks to you that I've made it this far!

@mikhailmoiseev
Copy link
Contributor

mikhailmoiseev commented Mar 7, 2025

That does not look like SV syntaxis error, more likely related to FPGA specific requirements. I would start to check if matrix_vector_matrix_btint_a is top module I/O port, and if so how this is mapped to FPGA pins.
Did you try a simple example like counter?

@libalis
Copy link
Author

libalis commented Mar 8, 2025

Thanks for your reply!
matrix_vector_matrix is an input.
I tried counter today, but similar errors occurred.
Do you happen to know what I should do in Vivado to wire the inputs correctly?

@mikhailmoiseev
Copy link
Contributor

I do not have expirience with Vivado. You need to go thorugh a Vivado example with a user guide. Google suggests "explicitly set the "IOSTANDARD" property for each port to a valid standard based on your board and design requirements."

@libalis
Copy link
Author

libalis commented Mar 9, 2025

Thanks for the suggestion!
I'll get right on it.

@libalis
Copy link
Author

libalis commented Mar 21, 2025

I’ve managed to generate the bitstream and even verified the functionality on an FPGA. Once again, a huge thank you for everything – without you, this would have simply been impossible. I do have two final questions though:

  1. While the results of the calculation are completely correct, the bit representation of the SystemC simulation differs from the VHDL behavior. Could this be due to the compiler optimizing the code or using an optimized version for translation?

  2. Are there any papers or resources that explain how the compiler works internally? It really is a masterpiece with everything it can do. Or could you perhaps briefly explain it to me?

Thanks again in advance!

@mikhailmoiseev
Copy link
Contributor

  1. Do you mean some wave forms differs in SystemC vs SV simulation? If so, that most likely caused by mistakes in the design.
  2. There are some explanations here https://github.com/intel/systemc-compiler/blob/main/doc/papers/icsc.pdf

@libalis
Copy link
Author

libalis commented Mar 22, 2025

  1. No, I didn't mean the waveform.
    I’m referring to the fact that when I read the output bits, the decimal number is the same, but the binary representation is different.
    This is possible because my ternary encoding has multiple possible representations for the same decimal number.
    But I don't understand why SystemC differs from SystemVerilog here.

  2. Thank you so much!

@mikhailmoiseev
Copy link
Contributor

In SystemC simulation native C++ types are used for <=64bit numbers.
For sc_bigint/sc_biguint >64bit there is bit-difference vs SystemVerilog.

@libalis
Copy link
Author

libalis commented Mar 23, 2025

Ah, I see, thank you very much!

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