Closed chhzh123 closed 4 years ago
Nice work. I suggest we make the following changes:
Performance section
Resource section
Great! The sw_exe
mode is kind of confusing to me (though I did it the same way in the Streaming Enhancement v2 PR). Actually we can run Vivado HLS with C-Sim, Synthesis, or Co-Sim modes. I think we can create a new option in the config
API to provide better control over the generated Tcl scripts.
It's indeed very odd that we are using the mode "sw_exe" here to represent HLS. What are the other options?
Also I suggest we change f.hls_report() to f.report(the_mode_we_set_in_the_target)
Something like (e.g. if we use a custom platform)
# set up a custom target
target = hcl.platform.custom(part="xcvu9p-fsgd2104-2-i")
# configure the platform with more control
tcl = open("xxx.tcl", "r").readlines()
target.config(
compile="vivado_hls",
mode="hw_sim", co_sim=True,
c_syn=True, Tcl=tcl)
# execution and test bench
f = hcl.build(s, target)
f(...)
# print report results
report = f.report()
It's indeed very odd that we are using the mode "sw_exe" here to represent HLS. What are the other options?
Yes, I have thought you have discussed and just follow the convention in the Streaming Enhancement v2 PR. Currently, HeteroCL supports sw_sim, hw_sim, hw_exe, and debug, but none of them target only HLS.
tcl = open("xxx.tcl", "r").readlines() target.config( compile="vivado_hls", mode="hw_sim", co_sim=True, c_syn=True, Tcl=tcl)
This is something we already support? If we allow the user to overwrite the Tcl script, we don't need them to specify other options c_syn and cosim. They should be already included in the custom script.
HeteroCL supports sw_sim, hw_sim, hw_exe, and debug, but none of them target only HLS.
I thought more about the "mode" string. Maybe we should simply allow the user to enter tool-specific options instead of trying to unify different ones from multiple tools. Say for Vivado HLS, let's just use "csyn", "csim", "cosim", plus the one for running vivado ("impl"? I forgot the name).
Default should be cysn. cosim should automatically invoke csyn. impl should always invoke "csyn". Of course, we should check the legality of the mode and prompt proper error messages.
Ideally, we should also allow them to enter multiple options such as mode="csim|cysn|cosim". Under the hood, we can use a bitmask to support these different options.
Does this make sense? This programming interface should be more intuitive and easier for us to maintain.
tcl = open("xxx.tcl", "r").readlines() target.config( compile="vivado_hls", mode="hw_sim", co_sim=True, c_syn=True, Tcl=tcl)
This is something we already support? If we allow the user to overwrite the Tcl script, we don't need them to specify other options c_syn and cosim. They should be already included in the custom script.
No. Agreed.
Agreed. That sounds better and more intuitive.
Yes, this makes sense to me. Should the original modes be removed? I'm not sure if this will conflict with other tools.
Yes, this makes sense to me. Should the original modes be removed? I'm not sure if this will conflict with other tools.
Let's update the modes for Vivado HLS first. Keep the current options for other tools for now. But we should eventually (hopefully quickly) phase out the old modes.
@Hecmay Shall we let @chhzh123 to give it a shot? or you want to work on this?
Let's update the modes for Vivado HLS first. Keep the current options for other tools for now. But we should eventually (hopefully quickly) phase out the old modes.
I can help fix the modes for Vivado HLS. For other tools, maybe I need @Hecmay 's support.
@chhzh123 Sounds good. Please give it a try.
Improvements are listed below:
+----------------+-----------------------------------+
| HLS Version | Vivado HLS 2018.2 |
| Product family | zynq |
| Target device | xc7z020clg484-1 |
| Top Model Name | test |
+----------------+-----------------------------------+
| Target CP | 10.00 ns |
| Estimated CP | 5.806 ns |
| Latency | Min 2623 cycles |
| | Max 2623 cycles |
| Interval | Min 2624 cycles |
| | Max 2624 cycles |
| Resources | Type Used Total Util |
| | -------- ------ ------- ------ |
| | BRAM_18K 2 280 1% |
| | DSP48E 0 220 0% |
| | FF 306 106400 0% |
| | LUT 498 53200 1% |
+----------------+-----------------------------------+
build_module.py
to runtime.py
.target.config(compile="vivado_hls",mode="csyn|cosim")
target.config(compile="vivado_hls",mode="cosim")
However, a warning message will be generated when executing the below code.
Warning: csyn needs to be done before cosim, so csyn is added to target mode.
Sounds good. We can merge Hongzheng’s PR first. I may add some other changes later in the next PR.
tcl
argument. For example,
script = open("myscript.tcl","r").read()
# the target mode will be blocked if tcl is set
target.config(compile="vivado_hls", mode="csyn", tcl=script)
I think we pretty much have every features we want in the function build interface, and it is ready to be merged. Do you have any other suggestions for the user interface? @zhangzhiru @seanlatias
I'm in favor.
One more nitpick on the latency report -- I suggest the following: Latency (cycles) | Min: 2623; Max: 2623
Same for interval.
Let's also find time to fix the reporting for other tools.
For Intel AOCL Profiler, they only have the report in form of HTML+JS, which makes it hard to interpret with scripts... But we can find out a way to solve it.
Need to fix:
target.config
-> report = f.report()
target.config(compile="vivado_hls", script=tcl)
hcl.build
Can you also add a "debug" mode to the VHLS runtime in another PR? That will be useful.
Can you also add a "debug" mode to the VHLS runtime in another PR? That will be useful.
Okay, what features should I add? Could you explain more about the "debug" mode?
It is used for inspecting the generated code (i.e. host code and device code). HeteroCL only returns code string in debug mode. You can refer to the example for SDAccel here: https://github.com/Hecmay/heterocl/blob/stream_to/tests/test_runtime_build.py#L39
In this PR, we introduce some APIs for users to check HLS reports from HeteroCL directly. To do synthesis, users need to set the target mode to
sw_exe
. #206 has added simple display of HLS reports, but this PR enhances it in the following ways:profile.json
to record reports, which is portable for users to do further analysis.f.hls_report()
API for users to retrieve the metadata. After users call the modulef
function to do simulation and synthesis,hls_report
can be called, and a dictionary is generated for users to access the data. For example,tvm/src/template/vivado/Makefile
has avivado
target. However, this target calls Vivado HLS instead of Vivado backend. Thus, we modify the target name tovivado_hls
.@zhangzhiru @Hecmay