diff --git a/docs/blog/2022_09_20_vunit_events.rst b/docs/blog/2022_09_20_vunit_events.rst index dd44b8960..9902ead05 100644 --- a/docs/blog/2022_09_20_vunit_events.rst +++ b/docs/blog/2022_09_20_vunit_events.rst @@ -330,7 +330,10 @@ use this example as a template for your own project. .. raw:: html :file: img/vunit_events/wait_statement_preprocessor.html -The preprocessor is added to the project using the :meth:`~vunit.ui.VUnit.add_preprocessor` method. The order number must be higher than that of the location preprocessor which is 1000 by default. This is to avoid unintended interference between the two. +The preprocessor is added to the project using the :meth:`~vunit.ui.VUnit.add_preprocessor` method. By setting the +order number lower than that of the location preprocessor (which has a default value of 100), the ``log_active`` +function is inserted first, allowing the location preprocessor to accurately pinpoint the source of the statement. + .. raw:: html :file: img/vunit_events/add_preprocessor.html diff --git a/docs/blog/img/vunit_events/add_preprocessor.html b/docs/blog/img/vunit_events/add_preprocessor.html index c5e6b1971..9f50c8286 100644 --- a/docs/blog/img/vunit_events/add_preprocessor.html +++ b/docs/blog/img/vunit_events/add_preprocessor.html @@ -1,4 +1,4 @@
vu = VUnit.from_argv()
-vu.enable_location_preprocessing()  # order = 1000 if no other value is provided
-vu.add_preprocessor(WaitStatementPreprocessor(order=1001))
+vu.enable_location_preprocessing()  # order = 100 if no other value is provided
+vu.add_preprocessor(WaitStatementPreprocessor(order=99))
 
diff --git a/docs/blog/img/vunit_events/log_for_check_latency_with_vunit_error.html b/docs/blog/img/vunit_events/log_for_check_latency_with_vunit_error.html index abb9dce65..e417f74ef 100644 --- a/docs/blog/img/vunit_events/log_for_check_latency_with_vunit_error.html +++ b/docs/blog/img/vunit_events/log_for_check_latency_with_vunit_error.html @@ -1,3 +1,3 @@ -
178000000 fs - vunit_lib:event_pkg          -    INFO - Event vunit_lib:vunit_error activated while waiting on "wait until is_active(new_data_set);"
+
178000000 fs - vunit_lib:event_pkg          -    INFO - Event vunit_lib:vunit_error activated while waiting on "wait until is_active(new_data_set);" (tb_event.vhd:417)
 178000000 fs - check                        -   ERROR - Equality check failed for #processed samples - Got 0000_0101 (5). Expected 10 (0000_1010). (tb_event.vhd:265)
 
diff --git a/docs/blog/img/vunit_events/log_with_core_dump.html b/docs/blog/img/vunit_events/log_with_core_dump.html index d67643aaa..f7ca31a1a 100644 --- a/docs/blog/img/vunit_events/log_with_core_dump.html +++ b/docs/blog/img/vunit_events/log_with_core_dump.html @@ -1,4 +1,4 @@ -
178000000 fs - vunit_lib:event_pkg          -    INFO - Event vunit_lib:vunit_error activated while waiting on "wait until is_active(new_data_set);"
+
178000000 fs - vunit_lib:event_pkg          -    INFO - Event vunit_lib:vunit_error activated while waiting on "wait until is_active(new_data_set);" (tb_event.vhd:417)
 178000000 fs - incrementer:core_dump        -    INFO - Control state is idle (incrementer.vhd:180)
 178000000 fs - incrementer:core_dump        -    INFO - Data processing state is idle (incrementer.vhd:181)
 178000000 fs - check                        -   ERROR - Equality check failed for #processed samples - Got 0000_0101 (5). Expected 10 (0000_1010). (tb_event.vhd:265)
diff --git a/docs/blog/img/vunit_events/wait_statement_preprocessor.html b/docs/blog/img/vunit_events/wait_statement_preprocessor.html
index 879561dd0..3b7306645 100644
--- a/docs/blog/img/vunit_events/wait_statement_preprocessor.html
+++ b/docs/blog/img/vunit_events/wait_statement_preprocessor.html
@@ -13,8 +13,9 @@
 
         # Regular expression finding wait statements on the form
         # wait [on sensitivity_list] [until condition] [for timeout];
+        # Any preceding text (prefix) is also picked-up. It will be examined later to exclude some special cases.
         self._wait_re = re.compile(
-            r"wait(\s+on\s+(?P<sensitivity_list>.*?))?(\s+until\s+(?P<condition>.*?))?(\s+for\s+(?P<timeout>.*?))?;",
+            r"(?P<prefix>^[^\r\n]*?)(?P<wait>wait)(\s+on\s+(?P<sensitivity_list>.*?))?(\s+until\s+(?P<condition>.*?))?(\s+for\s+(?P<timeout>.*?))?;",
             re.MULTILINE | re.DOTALL | re.IGNORECASE,
         )
 
@@ -30,24 +31,46 @@
         wait_statements.sort(key=lambda wait_statement: wait_statement.start(), reverse=True)
 
         for wait_statement in wait_statements:
+            prefix = wait_statement.group("prefix")
+            if prefix:
+                # Ignore commented statements wait looking statements in strings (not foolproof)
+                if ("--" in prefix) or ('"' in prefix):
+                    continue
+                # Remove any preceding statements but keep labels
+                prefix = prefix.split(";")[-1].lstrip()
+
             modified_wait_statement = "wait"
 
             # If the wait statement has an explicit sensitivity list (on ...), then vunit_error must be added to that
             sensitivity_list = wait_statement.group("sensitivity_list")
+            sensitivity_list_signals = []
             if sensitivity_list is not None:
-                new_sensitivity_list = f"{sensitivity_list}, vunit_error"
+                sensitivity_list_signals = [signal.strip() for signal in sensitivity_list.split(",")]
+                new_sensitivity_list = f"{', '.join(sensitivity_list_signals)}, vunit_error"
                 modified_wait_statement += f" on {new_sensitivity_list}"
 
             # Add log_active to an existing condition clause (until ...) or create one if not present
-            original_wait_statement = wait_statement.group(0)
+            original_wait_statement = wait_statement.group(0)[wait_statement.start("wait") - wait_statement.start() :]
             log_message = f'decorate("while waiting on ""{original_wait_statement}""")'
+            # The location preprocessor will not detect that the code in the message is quoted and it will modify
+            # any function it targets. is_active_msg is such a function but by appending a non-printable character
+            # to that function name we avoid this problem without altering the logged message
+            log_message = log_message.replace("is_active_msg", 'is_active_msg" & NUL & "')
             condition = wait_statement.group("condition")
             if condition is None:
-                new_condition = f"log_active(vunit_error, {log_message})"
+                # If there was a sensitivity list the VHDL event attribute of those signals must be in the
+                # condition or the wait statement will remain blocked on those VHDL events (log_active always
+                # returns false).
+                new_condition = " or ".join([f"{signal}'event" for signal in sensitivity_list_signals])
+                new_condition = new_condition + " or " if new_condition else new_condition
+                new_condition += f"log_active(vunit_error, {log_message})"
             elif "vunit_error" in condition:
                 continue  # Don't touch a wait statement already triggering on vunit_error
             else:
-                new_condition = f"({condition}) or log_active(vunit_error, {log_message})"
+                # The condition_operator function turns the original condition to a boolean that can be ORed
+                # with the boolean log_active function. Using the condition operator (??) doesn't work since it can't
+                # be applied to a condition that was already a boolean
+                new_condition = f"condition_operator({condition}) or log_active(vunit_error, {log_message})"
 
             modified_wait_statement += f" until {new_condition}"
 
@@ -59,7 +82,7 @@
             modified_wait_statement += ";"
 
             # Replace original wait statement
-            code = code[: wait_statement.start()] + modified_wait_statement + code[wait_statement.end() :]
+            code = code[: wait_statement.start("wait")] + modified_wait_statement + code[wait_statement.end() :]
 
         return code
 
diff --git a/docs/blog/src/vunit_events/run.py b/docs/blog/src/vunit_events/run.py index 9b9f5f52c..13f5659cf 100644 --- a/docs/blog/src/vunit_events/run.py +++ b/docs/blog/src/vunit_events/run.py @@ -30,8 +30,9 @@ def __init__(self, order): # Regular expression finding wait statements on the form # wait [on sensitivity_list] [until condition] [for timeout]; + # Any preceding text (prefix) is also picked-up. It will be examined later to exclude some special cases. self._wait_re = re.compile( - r"wait(\s+on\s+(?P.*?))?(\s+until\s+(?P.*?))?(\s+for\s+(?P.*?))?;", + r"(?P^[^\r\n]*?)(?Pwait)(\s+on\s+(?P.*?))?(\s+until\s+(?P.*?))?(\s+for\s+(?P.*?))?;", re.MULTILINE | re.DOTALL | re.IGNORECASE, ) @@ -47,24 +48,46 @@ def run(self, code, file_name): wait_statements.sort(key=lambda wait_statement: wait_statement.start(), reverse=True) for wait_statement in wait_statements: + prefix = wait_statement.group("prefix") + if prefix: + # Ignore commented statements wait looking statements in strings (not foolproof) + if ("--" in prefix) or ('"' in prefix): + continue + # Remove any preceding statements but keep labels + prefix = prefix.split(";")[-1].lstrip() + modified_wait_statement = "wait" # If the wait statement has an explicit sensitivity list (on ...), then vunit_error must be added to that sensitivity_list = wait_statement.group("sensitivity_list") + sensitivity_list_signals = [] if sensitivity_list is not None: - new_sensitivity_list = f"{sensitivity_list}, vunit_error" + sensitivity_list_signals = [signal.strip() for signal in sensitivity_list.split(",")] + new_sensitivity_list = f"{', '.join(sensitivity_list_signals)}, vunit_error" modified_wait_statement += f" on {new_sensitivity_list}" # Add log_active to an existing condition clause (until ...) or create one if not present - original_wait_statement = wait_statement.group(0) + original_wait_statement = wait_statement.group(0)[wait_statement.start("wait") - wait_statement.start() :] log_message = f'decorate("while waiting on ""{original_wait_statement}""")' + # The location preprocessor will not detect that the code in the message is quoted and it will modify + # any function it targets. is_active_msg is such a function but by appending a non-printable character + # to that function name we avoid this problem without altering the logged message + log_message = log_message.replace("is_active_msg", 'is_active_msg" & NUL & "') condition = wait_statement.group("condition") if condition is None: - new_condition = f"log_active(vunit_error, {log_message})" + # If there was a sensitivity list the VHDL event attribute of those signals must be in the + # condition or the wait statement will remain blocked on those VHDL events (log_active always + # returns false). + new_condition = " or ".join([f"{signal}'event" for signal in sensitivity_list_signals]) + new_condition = new_condition + " or " if new_condition else new_condition + new_condition += f"log_active(vunit_error, {log_message})" elif "vunit_error" in condition: continue # Don't touch a wait statement already triggering on vunit_error else: - new_condition = f"({condition}) or log_active(vunit_error, {log_message})" + # The condition_operator function turns the original condition to a boolean that can be ORed + # with the boolean log_active function. Using the condition operator (??) doesn't work since it can't + # be applied to a condition that was already a boolean + new_condition = f"condition_operator({condition}) or log_active(vunit_error, {log_message})" modified_wait_statement += f" until {new_condition}" @@ -76,7 +99,7 @@ def run(self, code, file_name): modified_wait_statement += ";" # Replace original wait statement - code = code[: wait_statement.start()] + modified_wait_statement + code[wait_statement.end() :] + code = code[: wait_statement.start("wait")] + modified_wait_statement + code[wait_statement.end() :] return code @@ -109,16 +132,16 @@ class RestrictedWaitStatementPreprocessor(WaitStatementPreprocessor): def __init__(self, order): super().__init__(order) self._wait_re = re.compile( - r"(?<=preprocess_this : )wait(\s+on\s+(?P.*?))?(\s+until\s+(?P.*?))?(\s+for\s+(?P.*?))?;", + r"(?<=preprocess_this :)(?P\s)(?Pwait)(\s+on\s+(?P.*?))?(\s+until\s+(?P.*?))?(\s+for\s+(?P.*?))?;", re.MULTILINE | re.DOTALL | re.IGNORECASE, ) - vu.add_preprocessor(RestrictedWaitStatementPreprocessor(order=1001)) + vu.add_preprocessor(RestrictedWaitStatementPreprocessor(order=99)) else: # For blog # start_snippet add_preprocessor vu = VUnit.from_argv() - vu.enable_location_preprocessing() # order = 1000 if no other value is provided - vu.add_preprocessor(WaitStatementPreprocessor(order=1001)) + vu.enable_location_preprocessing() # order = 100 if no other value is provided + vu.add_preprocessor(WaitStatementPreprocessor(order=99)) # end_snippet add_preprocessor root = Path(__file__).parent diff --git a/vunit/vhdl/data_types/src/event_pkg.vhd b/vunit/vhdl/data_types/src/event_pkg.vhd index 620091293..c4c3195fb 100644 --- a/vunit/vhdl/data_types/src/event_pkg.vhd +++ b/vunit/vhdl/data_types/src/event_pkg.vhd @@ -45,22 +45,22 @@ package event_pkg is impure function name(signal event : any_event_t) return string; impure function full_name(signal event : any_event_t) return string; - -- Log a message if event is active in current delta cycle but always returns false. - -- The function supports message decoration and will add "Event activated" - -- before the custom message. + -- Return true if event is active in current delta cycle, false otherwise. If true it will + -- also produce a message. The function supports message decoration and will add + -- "Event activated" before the custom message. -- - -- log_active is typically used in wait statements to report exceptions to the + -- is_active_msg is typically used in wait statements to handle and report exceptions to the -- expected program flow. For example, -- - -- wait until some_test_condition or log_active(test_runner_watchdog_timeout, decorate("while waiting for some test condition"); + -- wait until some_test_condition or is_active_msg(test_runner_watchdog_timeout, decorate("while waiting for some test condition"); -- - -- log_active supports log location which means that if VHDL-2019 is used, or if location preprocessing is enabled, the following + -- is_active_msg supports log location which means that if VHDL-2019 is used, or if location preprocessing is enabled, the following -- is usually sufficient for debugging: -- - -- wait until some_test_condition or log_active(test_runner_watchdog_timeout) + -- wait until some_test_condition or is_active_msg(test_runner_watchdog_timeout) -- -- The caller can also provide a custom logger. - impure function log_active( + impure function is_active_msg( signal event : any_event_t; constant msg : in string := decorate_tag; constant log_level : in log_level_t := info; @@ -70,8 +70,7 @@ package event_pkg is constant file_name : in string := "" ) return boolean; - -- Produce a message just like log_active but return true if the event is active. - impure function is_active_msg( + impure function log_active( signal event : any_event_t; constant msg : in string := decorate_tag; constant log_level : in log_level_t := info; @@ -81,6 +80,21 @@ package event_pkg is constant file_name : in string := "" ) return boolean; + -- condition_operator is a VHDL-93 compatible function equivalent to the + -- condition operator (??) with the difference that it is also defined + -- for boolean values to simplify some code generation tasks. + function condition_operator( + value : boolean + ) return boolean; + + function condition_operator( + value : bit + ) return boolean; + + function condition_operator( + value : std_ulogic + ) return boolean; + end package; package body event_pkg is @@ -206,4 +220,25 @@ package body event_pkg is return false; end; + function condition_operator( + value : boolean + ) return boolean is + begin + return value; + end; + + function condition_operator( + value : bit + ) return boolean is + begin + return value = '1'; + end; + + function condition_operator( + value : std_ulogic + ) return boolean is + begin + return to_x01(value) = '1'; + end; + end package body; diff --git a/vunit/vhdl/data_types/test/tb_event_pkg.vhd b/vunit/vhdl/data_types/test/tb_event_pkg.vhd index c4600c787..930595d28 100644 --- a/vunit/vhdl/data_types/test/tb_event_pkg.vhd +++ b/vunit/vhdl/data_types/test/tb_event_pkg.vhd @@ -199,6 +199,22 @@ begin end loop; wait for 1 ps; check_equal(get(number, number_observed_idx), get(number, number_produced_idx)); + + elsif run("Test condition_operator function") then + check_true(condition_operator(bit'('1'))); + check_true(condition_operator(std_ulogic'('1'))); + check_true(condition_operator(std_ulogic'('H'))); + check_true(condition_operator(true)); + + check_false(condition_operator(bit'('0'))); + check_false(condition_operator(std_ulogic'('0'))); + check_false(condition_operator(std_ulogic'('L'))); + check_false(condition_operator(std_ulogic'('X'))); + check_false(condition_operator(std_ulogic'('U'))); + check_false(condition_operator(std_ulogic'('W'))); + check_false(condition_operator(std_ulogic'('-'))); + check_false(condition_operator(std_ulogic'('Z'))); + check_false(condition_operator(false)); end if; end loop;