HLS check for critical warnings hits with false positive
Closed this issue · 5 comments
The actions/hls.mk
Makefile checks for CRITICAL WARNING
s appeared during the C synthesis phase with a simple grep
:
Line 81 in 704a558
However, this won't work in the first place: CRITICAL WARNING
s are spelled all caps in HLS logs, and you'll need a -i
if both cases are to be matched. For example:
CRITICAL WARNING: [RTGEN 206-102] Illegal connection is found on FIFO pin 'top|ovflo' connecting to 'stg_356'('top_fft_top|ovflo').
(from https://www.xilinx.com/support/answers/67756.html)
And, what's more, this matching of critical
will cause problems when the design failed to meet clocking requirements specified by the user. In such situations, Vivado HLS will report the operations on the critical path. Such messages will look like this:
WARNING: [SCHED 204-70] Cannot meet target clock period from 'call' operation ('call_ret62') to '<redacted>' to 'call' operation ('call_ret') to '<redacted>' (combination delay: 7.604 ns) to honor II or Latency constraint in region 'Loop 25'.
WARNING: [SCHED 204-21] Estimated clock period (7.604ns) exceeds the target (target clock period: 4ns, clock uncertainty: 0.5ns, effective delay budget: 3.5ns).
WARNING: [SCHED 204-21] The critical path consists of the following:
'call' operation ('call_ret56', <redacted>.cpp:132) to '<redacted>' (3.04 ns)
'mux' operation ('ctx_S_0_96_1', <redacted>.cpp:132) (1.07 ns)
'call' operation ('call_ret57', <redacted>.cpp:138) to '<redacted>' (3.5 ns)
And the critical
word gets matched by mistake, making a non-critical warning kill the build.
The fix would be using CRITICAL
to match the critical warnings instead of critical
.
Hi @luyong6 and @KireinaHoro
In hls.mk, the goal of the test was only looking for the following message WARNING: [SCHED 204-21] The critical path....
since it was seen that if this timing issue was faced in HLS, then we always face timing issues later in routing phase.
If you want to add tracking of CRITICAL WARNING
, then I'll use your suggestion adding the -i
option to the grep.
Correction pushed into master. @KireinaHoro , can we close the issue ?
ok thanks. If any issue, please reopen it