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.

Sign up for GitHub

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

RAD-Sim Design Correctness Reporting #22

Merged
merged 5 commits into from
Dec 1, 2023
Merged

RAD-Sim Design Correctness Reporting #22

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2b4f324
Added Exit Code Reporting for RAD-Sim Modules. Intentionally added an…
geotrieu Nov 28, 2023
7b7f3f1
Edited config.py to use new exit code function
geotrieu Nov 28, 2023
3442dd9
Removed error after verifying error is picked up by CI
geotrieu Nov 28, 2023
699993c
Changed DesignResult name to Sim Exit Code
geotrieu Nov 30, 2023
ec92d85
Renamed GetDesignExitCode to GetSimExitCode
geotrieu Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rad-sim/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 0;\n")
main_cpp_file.write("\treturn radsim_design.GetDesignResult();\n")
main_cpp_file.write("}\n")

def prepare_build_dir(design_name):
Expand Down
8 changes: 6 additions & 2 deletions rad-sim/example-designs/add/add_driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@ void add_driver::sink() {
//std::cout << "Received " << response.read().to_uint64() << " sum from the adder!" << std::endl;
//std::cout << "The actual sum is " << actual_sum << std::endl;

if (response.read() != actual_sum) std::cout << "FAILURE - Output is not matching!" << std::endl;
else std::cout << "SUCCESS - Output is matching!" << std::endl;
if (response.read() != actual_sum) {
std::cout << "FAILURE - Output is not matching!" << std::endl;
radsim_design.ReportDesignFailure();
} else {
std::cout << "SUCCESS - Output is matching!" << std::endl;
}

end_cycle = GetSimulationCycle(1.0);
end_time = std::chrono::steady_clock::now();
Expand Down
4 changes: 2 additions & 2 deletions rad-sim/example-designs/dlrm/dlrm_driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ void dlrm_driver::sink() {
if (all_outputs_matching) {
std::cout << "Simulation PASSED! All outputs matching!" << std::endl;
} else {
std::cout << "Simulation FAILED! Some outputs are NOT matching!"
<< std::endl;
std::cout << "Simulation FAILED! Some outputs are NOT matching!" << std::endl;
radsim_design.ReportDesignFailure();
}
_end_cycle =
GetSimulationCycle(radsim_config.GetDoubleKnob("sim_driver_period"));
Expand Down
5 changes: 4 additions & 1 deletion rad-sim/example-designs/mlp/mlp_driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ void mlp_driver::sink() {
}
wait();
}
if (mistake) std::cout << "FAILURE - Some outputs NOT matching!" << std::endl;
if (mistake) {
std::cout << "FAILURE - Some outputs NOT matching!" << std::endl;
radsim_design.ReportDesignFailure();
}
else std::cout << "SUCCESS - All outputs are matching!" << std::endl;

end_cycle = GetSimulationCycle(1.0);
Expand Down
9 changes: 7 additions & 2 deletions rad-sim/example-designs/mlp_int8/mlp_driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,14 +307,19 @@ void mlp_driver::sink() {
}
wait();
}
if (mistake) std::cout << "FAILURE - Some outputs NOT matching!" << std::endl;
else std::cout << "SUCCESS - All outputs are matching!" << std::endl;
if (mistake) {
std::cout << "FAILURE - Some outputs NOT matching!" << std::endl;
radsim_design.ReportDesignFailure();
} else {
std::cout << "SUCCESS - All outputs are matching!" << std::endl;
}

end_cycle = GetSimulationCycle(1.0);
end_time = std::chrono::steady_clock::now();
std::cout << "Simulation Cycles = " << end_cycle - start_cycle << std::endl;
std::cout << "Simulation Time = " << std::chrono::duration_cast<std::chrono::milliseconds> (end_time - start_time).count() << " ms" << std::endl;
NoCTransactionTelemetry::DumpStatsToFile("stats.csv");
NoCFlitTelemetry::DumpNoCFlitTracesToFile("flit_traces.csv");

std::vector<double> aggregate_bandwidths = NoCTransactionTelemetry::DumpTrafficFlows("traffic_flows",
end_cycle - start_cycle, radsim_design.GetNodeModuleNames());
Expand Down
8 changes: 6 additions & 2 deletions rad-sim/example-designs/rtl_add/rtl_add_driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@ void rtl_add_driver::sink() {
//std::cout << "Received " << response.read().to_uint64() << " sum from the adder!" << std::endl;
//std::cout << "The actual sum is " << actual_sum << std::endl;

if (response.read() != actual_sum) std::cout << "FAILURE - Output is not matching!" << std::endl;
else std::cout << "SUCCESS - Output is matching!" << std::endl;
if (response.read() != actual_sum) {
std::cout << "FAILURE - Output is not matching!" << std::endl;
radsim_design.ReportDesignFailure();
} else {
std::cout << "SUCCESS - Output is matching!" << std::endl;
}

end_cycle = GetSimulationCycle(1.0);
end_time = std::chrono::steady_clock::now();
Expand Down
8 changes: 8 additions & 0 deletions rad-sim/sim/design_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, will change

return _design_result;
}

void RADSimDesignContext::ReportDesignFailure() {
_design_result = 1;
}
5 changes: 5 additions & 0 deletions rad-sim/sim/design_context.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

class RADSimDesignContext {
private:
int _design_result = 0;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


std::vector<sc_clock *> _noc_clks;
std::vector<sc_clock *> _adapter_clks;
std::vector<sc_clock *> _module_clks;
Expand Down Expand Up @@ -96,6 +98,9 @@ class RADSimDesignContext {
void DumpDesignContext();
std::vector<std::vector<std::set<std::string>>> &GetNodeModuleNames();
uint64_t GetPortBaseAddress(std::string &port_name);

int GetDesignResult();
void ReportDesignFailure();
};

extern RADSimDesignContext radsim_design;
2 changes: 1 addition & 1 deletion rad-sim/sim/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ int sc_main(int argc, char *argv[]) {
sim_trace_probe.dump_traces();
(void)argc;
(void)argv;
return 0;
return radsim_design.GetDesignResult();
}
Loading