open-power/snap

HLS check for critical warnings hits with false positive

Closed this issue · 5 comments

The actions/hls.mk Makefile checks for CRITICAL WARNINGs appeared during the C synthesis phase with a simple grep:

@grep -A8 critical $(SOLUTION_DIR)*/$(SOLUTION_NAME)/$(SOLUTION_NAME).log ; \

However, this won't work in the first place: CRITICAL WARNINGs 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.

Fix this in #842

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