RAD-Sim Design Correctness Reporting #22
Conversation
… error to test CI
rad-sim/sim/design_context.cpp
Outdated
| @@ -688,4 +688,12 @@ uint64_t RADSimDesignContext::GetPortBaseAddress(std::string &port_name) { | |||
| assert(_aximm_port_base_addresses.find(port_name) != | |||
| _aximm_port_base_addresses.end()); | |||
| return _aximm_port_base_addresses[port_name]; | |||
| } | |||
|
|
|||
| int RADSimDesignContext::GetDesignResult() { | |||
There was a problem hiding this comment.
I think the name _design_result might not be very intuitive. If this is meant to only report the success and failure of the simulation, we should name it something like _sim_exit_code (0 is success and 1 is failure).
There was a problem hiding this comment.
I agree, will change
rad-sim/sim/design_context.hpp
Outdated
| @@ -12,6 +12,8 @@ | |||
|
|
|||
| class RADSimDesignContext { | |||
| private: | |||
| int _design_result = 0; | |||
There was a problem hiding this comment.
Same comment about the name. I think you should also write a comment to explain what this variable is and what different values mean (0 is success and 1 is failure, maybe other outcomes can be added later if needed).
There was a problem hiding this comment.
Is design_result the number of failures? If so, better to rename it. If it can only be one or zero, better to make it a bool.
There was a problem hiding this comment.
I can imagine other exit codes than just 0 or 1 to report different failure reasons (e.g. simulation timeout after some number of cycles). Not sure though. That's why my earlier comment suggested renaming to _sim_exit_code and commenting what each code means.
There was a problem hiding this comment.
I thought about this initially and had it as bool. But I came to the same conclusion as Andrew, yes - the exit code could be anything. Right now, we have it only as 0 or 1, thus there is a specific function to report a failure instead of a general function to set a result. It wouldn't be hard to add that functionality in the future if we wanted to report other exit codes.
There was a problem hiding this comment.
Is design_result the number of failures? If so, better to rename it. If it can only be one or zero, better to make it a bool.
It is not the number of failures, it is just the exit code. However, it could be any signed integer, so it could represent something down the line.
rad-sim/config.py
Outdated
| @@ -294,7 +294,7 @@ def generate_radsim_main(design_name): | |||
| main_cpp_file.write("\tsim_trace_probe.dump_traces();\n") | |||
| main_cpp_file.write("\t(void)argc;\n") | |||
| main_cpp_file.write("\t(void)argv;\n") | |||
| main_cpp_file.write("\treturn radsim_design.GetDesignResult();\n") | |||
| main_cpp_file.write("\treturn radsim_design.GetDesignExitCode();\n") | |||
There was a problem hiding this comment.
Why aren't we naming this function GetSimExitCode instead of GetDesignExitCode?
Introduces the
ReportDesignFailure()function in the RAD-Sim Design Context to report a design failure (i.e. when the design fails the test bench).This is an optional call, if the function is not called, success is assumed and the original behaviour is preserved.
The design failure is returned as an exit code from the
sc_mainfunction.This PR is a branch of the
dev-gtrieubranch to observe the correct diffs. Please merge #21 before this, and change the base tomain.