Support command-line overriding `define (#5900) (#5908)

docs/CONTRIBUTORS

@@ -26,6 +26,7 @@ Arkadiusz Kozdra
 Arthur Rosa
 Aylon Chaim Porat
 Bartłomiej Chmiel
+Brian Li
 Cameron Kirk
 Chih-Mao Chen
 Chris Bachhuber

docs/guide/warnings.rst

@@ -472,6 +472,30 @@ List Of Warnings
    correctly.
 
 
+.. option:: DEFOVERRIDE
+
+   Warns that a macro definition within the code is being overridden by a command line directive:
+
+   For example, running Verilator with :code:`<+define+\<DUP\>=\<def2\>>` and
+
+   .. code-block:: sv
+      :linenos:
+      :emphasize-lines: 1
+
+         `define DUP def2 //<--- Warning
+
+   Results in:
+
+   .. code-block::
+         %Warning-DEFOVERRIDE: example.v1:20: Overriding define: 'DEF' with value: 'def2' to existing command line define value: 'def1'
+                      ... Location of previous definition, with value: '50'
+
+   While not explicitly stated in the IEEE 1800-2023 standard, this warning
+   tracks with the other simulators' behavior of overriding macro
+   definitions within code files with the definition passed in through
+   the command line.
+
+
 .. option:: DEFPARAM
 
    Warns that the :code:`defparam` statement was deprecated in IEEE 1364-2001,
@@ -1261,6 +1285,7 @@ List Of Warnings
    Ignoring this warning will only suppress the lint check; it will
    simulate correctly.
 
+
 .. option:: PINCONNECTEMPTY
 
    .. TODO better example

src/V3Error.h

@@ -90,6 +90,7 @@ public:
         CONTASSREG,     // Continuous assignment on reg
         COVERIGN,       // Coverage ignored
         DECLFILENAME,   // Declaration doesn't match filename
+        DEFOVERRIDE,    // Overriding existing define macro through command line
         DEFPARAM,       // Style: Defparam
         DEPRECATED,     // Feature will be deprecated
         ENCAPSULATED,   // Error: local/protected violation
@@ -196,7 +197,7 @@ public:
             "BLKANDNBLK", "BLKLOOPINIT", "BLKSEQ", "BSSPACE",
             "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CASTCONST", "CDCRSTLOGIC", "CLKDATA",
             "CMPCONST", "COLONPLUS", "COMBDLY", "CONSTRAINTIGN", "CONTASSREG", "COVERIGN",
-            "DECLFILENAME", "DEFPARAM", "DEPRECATED",
+            "DECLFILENAME", "DEFOVERRIDE", "DEFPARAM", "DEPRECATED",
             "ENCAPSULATED", "ENDLABEL", "ENUMVALUE", "EOFNEWLINE", "GENCLK",
             "GENUNNAMED", "HIERBLOCK",
             "IFDEPTH", "IGNOREDRETURN",

src/V3PreProc.cpp

@@ -211,6 +211,7 @@ private:
     string defineSubst(VDefineRef* refp);
 
     bool defExists(const string& name);
+    bool defCmdline(const string& name);
     string defValue(const string& name);
     string defParams(const string& name);
     FileLine* defFileline(const string& name);
@@ -316,6 +317,13 @@ bool V3PreProcImp::defExists(const string& name) {
     const auto iter = m_defines.find(name);
     return (iter != m_defines.end());
 }
+bool V3PreProcImp::defCmdline(const string& name) {
+    const auto iter = m_defines.find(name);
+    if (iter == m_defines.end()) {
+        return false;
+    }
+    return iter->second.cmdline();
+}
 string V3PreProcImp::defValue(const string& name) {
     const auto iter = m_defines.find(name);
     if (iter == m_defines.end()) {
@@ -345,21 +353,38 @@ void V3PreProcImp::define(FileLine* fl, const string& name, const string& value,
                                                                   << "' (IEEE 1800-2023 22.5.1)");
     } else {
         if (defExists(name)) {
-            if (!(defValue(name) == value
-                  && defParams(name) == params)) {  // Duplicate defs are OK
-                fl->v3warn(REDEFMACRO, "Redefining existing define: '"
-                                           << name << "', with different value: '" << value
-                                           << (params == "" ? "" : " ") << params << "'\n"
-                                           << fl->warnContextPrimary() << '\n'
-                                           << defFileline(name)->warnOther()
-                                           << "... Location of previous definition, with value: '"
-                                           << defValue(name)
-                                           << (defParams(name).empty() ? "" : " ")
-                                           << defParams(name) << "'\n"
-                                           << defFileline(name)->warnContextSecondary());
+            if (defCmdline(name) && !cmdline) {
+                fl->v3warn(DEFOVERRIDE, "Overriding define: '"
+                                        << name << "' with value: '" << value
+                                        << "' to existing command line define value: '"
+                                        << defValue(name)
+                                        << (params == "" ? "" : " ") << params << "'\n"
+                                        << fl->warnContextPrimary() << '\n'
+                                        << defFileline(name)->warnOther()
+                                        << "... Location of previous definition, with value: '"
+                                        << defValue(name)
+                                        << (defParams(name).empty() ? "" : " ")
+                                        << defParams(name) << "'\n"
+                                        << defFileline(name)->warnContextSecondary());
+                return;
+            } else {
+                if (!(defValue(name) == value
+                    && defParams(name) == params)) {  // Duplicate defs are OK
+                    fl->v3warn(REDEFMACRO, "Redefining existing define: '"
+                                            << name << "', with different value: '" << value
+                                            << (params == "" ? "" : " ") << params << "'\n"
+                                            << fl->warnContextPrimary() << '\n'
+                                            << defFileline(name)->warnOther()
+                                            << "... Location of previous definition, with value: '"
+                                            << defValue(name)
+                                            << (defParams(name).empty() ? "" : " ")
+                                            << defParams(name) << "'\n"
+                                            << defFileline(name)->warnContextSecondary());
+                }
+                undef(name);
             }
-            undef(name);
         }
+
         m_defines.emplace(name, VDefine{fl, value, params, cmdline});
     }
 }

test_regress/t/t_define_override.out

@@ -0,0 +1,9 @@
+%Warning-REDEFMACRO: Redefining existing define: 'TEST_MACRO', with different value: '50'
+                     ... Location of previous definition, with value: '20'
+                     ... For warning description see https://verilator.org/warn/REDEFMACRO?v=latest
+                     ... Use "/* verilator lint_off REDEFMACRO */" and lint_on around source to disable this message.
+%Warning-DEFOVERRIDE: t/t_define_override.v:9:23: Overriding define: 'TEST_MACRO' with value: '10' to existing command line define value: '50'
+                      ... Location of previous definition, with value: '50'
+%Warning-DEFOVERRIDE: t/t_define_override.v:10:24: Overriding define: 'TEST_MACRO' with value: '100' to existing command line define value: '50'
+                      ... Location of previous definition, with value: '50'
+%Error: Exiting due to

test_regress/t/t_define_override.py

@@ -0,0 +1,16 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2025 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+import vltest_bootstrap
+
+test.scenarios('linter')
+
+test.lint(verilator_flags2=["+define+TEST_MACRO=20 +define+TEST_MACRO=50"], fails=True, expect_filename=test.golden_filename)
+
+test.passes()

test_regress/t/t_define_override.v

@@ -0,0 +1,20 @@
+// DESCRIPTION: Verilator: Multiple `defines while using +define+
+// as a command-line argument as well
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2025 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+`define TEST_MACRO 10
+`define TEST_MACRO 100
+
+`define STRINGIFY(x) `"x`"
+
+module test (
+);
+    initial begin
+        $display("TEST_MACRO %s", `STRINGIFY(`TEST_MACRO));
+        $finish;
+    end
+
+endmodule

test_regress/t/t_define_override_empty.out

@@ -0,0 +1 @@
+TEST_MACRO  

test_regress/t/t_define_override_empty.py

@@ -0,0 +1,21 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2025 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+import vltest_bootstrap
+
+test.scenarios('linter')
+test.top_filename = "t/t_define_override.v"
+
+#test.lint(verilator_flags2=["+define+TEST_MACRO"], fails=True, expect_filename=test.golden_filename)
+
+test.compile(verilator_flags2=["-Wno-DEFOVERRIDE -Wno-REDEFMACRO +define+TEST_MACRO"])
+
+test.execute(expect_filename=test.golden_filename)
+
+test.passes()

test_regress/t/t_define_override_output.out

@@ -0,0 +1 @@
+TEST_MACRO 50

test_regress/t/t_define_override_output.py

@@ -0,0 +1,19 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2025 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+import vltest_bootstrap
+
+test.scenarios('vlt')
+test.top_filename = "t/t_define_override.v"
+
+test.compile(verilator_flags2=["-Wno-DEFOVERRIDE -Wno-REDEFMACRO +define+TEST_MACRO=20 +define+TEST_MACRO=50"])
+
+test.execute(expect_filename=test.golden_filename)
+
+test.passes()