Add support for SystemVerilog-style `define to Verilog frontend
authorRupert Swarbrick <rswarbrick@lowrisc.org>
Tue, 17 Mar 2020 09:34:31 +0000 (09:34 +0000)
committerRupert Swarbrick <rswarbrick@gmail.com>
Fri, 27 Mar 2020 16:08:26 +0000 (16:08 +0000)
This patch should support things like

  `define foo(a, b = 3, c)   a+b+c

  `foo(1, ,2)

which will evaluate to 1+3+2. It also spots mistakes like

  `foo(1)

(the 3rd argument doesn't have a default value, so a call site is
required to set it).

Most of the patch is a simple parser for the format in preproc.cc, but
I've also taken the opportunity to wrap up the "name -> definition"
map in a type, rather than use multiple std::map's.

Since this type needs to be visible to code that touches defines, I've
pulled it (and the frontend_verilog_preproc declaration) out into a
new file at frontends/verilog/preproc.h and included that where
necessary.

Finally, the patch adds a few tests in tests/various to check that we
are parsing everything correctly.

frontends/verilog/preproc.cc
frontends/verilog/preproc.h [new file with mode: 0644]
frontends/verilog/verilog_frontend.cc
frontends/verilog/verilog_frontend.h
kernel/rtlil.cc
kernel/rtlil.h
passes/cmds/design.cc
tests/various/sv_defines.ys [new file with mode: 0644]
tests/various/sv_defines_dup.ys [new file with mode: 0644]
tests/various/sv_defines_mismatch.ys [new file with mode: 0644]
tests/various/sv_defines_too_few.ys [new file with mode: 0644]

index 161253a99fec97c080b2aeb1aea5c48ff9fd2601..7905ea598e6bbe2bafacf8bea4e239509b03fbfa 100644 (file)
  *
  */
 
+#include "preproc.h"
 #include "verilog_frontend.h"
 #include "kernel/log.h"
+#include <assert.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <string.h>
@@ -199,6 +201,175 @@ static std::string next_token(bool pass_newline = false)
        return token;
 }
 
+struct macro_arg_t
+{
+       macro_arg_t(const std::string &name_, const char *default_value_)
+               : name(name_),
+                 has_default(default_value_ != nullptr),
+                 default_value(default_value_ ? default_value_ : "")
+       {}
+
+       std::string name;
+       bool        has_default;
+       std::string default_value;
+};
+
+static bool all_white(const std::string &str)
+{
+       for (char c : str)
+               if (!isspace(c))
+                       return false;
+       return true;
+}
+
+struct arg_map_t
+{
+       arg_map_t()
+       {}
+
+       void add_arg(const std::string &name, const char *default_value)
+       {
+               if (find(name)) {
+                       log_error("Duplicate macro arguments with name `%s'.\n", name.c_str());
+               }
+
+               name_to_pos[name] = args.size();
+               args.push_back(macro_arg_t(name, default_value));
+       }
+
+       // Find an argument by name; return nullptr if it doesn't exist. If pos is not null, write
+       // the argument's position to it on success.
+       const macro_arg_t *find(const std::string &name, int *pos = nullptr) const
+       {
+               auto it = name_to_pos.find(name);
+               if (it == name_to_pos.end())
+                       return nullptr;
+
+               if (pos) *pos = it->second;
+               return &args[it->second];
+       }
+
+       // Construct the name for the local macro definition we use for the given argument
+       // (something like macro_foobar_arg2). This doesn't include the leading backtick.
+       static std::string str_token(const std::string &macro_name, int pos)
+       {
+               return stringf("macro_%s_arg%d", macro_name.c_str(), pos);
+       }
+
+       // Return definitions for the macro arguments (so that substituting in the macro body and
+       // then performing macro expansion will do argument substitution properly).
+       std::vector<std::pair<std::string, std::string>>
+       get_vals(const std::string &macro_name, const std::vector<std::string> &arg_vals) const
+       {
+               std::vector<std::pair<std::string, std::string>> ret;
+               for (int i = 0; i < GetSize(args); ++ i) {
+                       // The SystemVerilog rules are:
+                       //
+                       //   - If the call site specifies an argument and it's not whitespace, use
+                       //     it.
+                       //
+                       //   - Otherwise, if the argument has a default value, use it.
+                       //
+                       //   - Otherwise, if the call site specified whitespace, use that.
+                       //
+                       //   - Otherwise, error.
+                       const std::string *dflt = nullptr;
+                       if (args[i].has_default)
+                               dflt = &args[i].default_value;
+
+                       const std::string *given = nullptr;
+                       if (i < GetSize(arg_vals))
+                               given = &arg_vals[i];
+
+                       const std::string *val = nullptr;
+                       if (given && (! (dflt && all_white(*given))))
+                               val = given;
+                       else if (dflt)
+                               val = dflt;
+                       else if (given)
+                               val = given;
+                       else
+                               log_error("Cannot expand macro `%s by giving only %d argument%s "
+                                         "(argument %d has no default).\n",
+                                         macro_name.c_str(), GetSize(arg_vals),
+                                         (GetSize(arg_vals) == 1 ? "" : "s"), i + 1);
+
+                       assert(val);
+                       ret.push_back(std::make_pair(str_token(macro_name, i), * val));
+               }
+               return ret;
+       }
+
+
+       std::vector<macro_arg_t>   args;
+       std::map<std::string, int> name_to_pos;
+};
+
+struct define_body_t
+{
+       define_body_t(const std::string &body, const arg_map_t *args = nullptr)
+         : body(body),
+           has_args(args != nullptr),
+           args(args ? *args : arg_map_t())
+       {}
+
+       std::string body;
+       bool        has_args;
+       arg_map_t   args;
+};
+
+define_map_t::define_map_t()
+{
+       add("YOSYS", "1");
+       add(formal_mode ? "FORMAL" : "SYNTHESIS", "1");
+}
+
+// We must define this destructor here (rather than relying on the default), because we need to
+// define it somewhere we've got a complete definition of define_body_t.
+define_map_t::~define_map_t()
+{}
+
+void
+define_map_t::add(const std::string &name, const std::string &txt, const arg_map_t *args)
+{
+       defines[name] = std::unique_ptr<define_body_t>(new define_body_t(txt, args));
+}
+
+void define_map_t::merge(const define_map_t &map)
+{
+       for (const auto &pr : map.defines) {
+               // These contortions are so that we take a copy of each definition body in
+               // map.defines.
+               defines[pr.first] = std::unique_ptr<define_body_t>(new define_body_t(*pr.second));
+       }
+}
+
+const define_body_t *define_map_t::find(const std::string &name) const
+{
+       auto it = defines.find(name);
+       return (it == defines.end()) ? nullptr : it->second.get();
+}
+
+void define_map_t::erase(const std::string &name)
+{
+       defines.erase(name);
+}
+
+void define_map_t::clear()
+{
+       defines.clear();
+}
+
+void define_map_t::log() const
+{
+       for (auto &it : defines) {
+               const std::string &name = it.first;
+               const define_body_t &body = *it.second;
+               Yosys::log("`define %s%s %s\n",
+                          name.c_str(), body.has_args ? "()" : "", body.body.c_str());
+       }
+}
+
 static void input_file(std::istream &f, std::string filename)
 {
        char buffer[513];
@@ -215,11 +386,59 @@ static void input_file(std::istream &f, std::string filename)
        input_buffer.insert(it, "\n`file_pop\n");
 }
 
+// Read tokens to get one argument (either a macro argument at a callsite or a default argument in a
+// macro definition). Writes the argument to dest. Returns true if we finished with ')' (the end of
+// the argument list); false if we finished with ','.
+static bool read_argument(std::string &dest)
+{
+       std::vector<char> openers;
+       for (;;) {
+               skip_spaces();
+               std::string tok = next_token(true);
+               if (tok == ")") {
+                       if (openers.empty())
+                               return true;
+                       if (openers.back() != '(')
+                               log_error("Mismatched brackets in macro argument: %c and %c.\n",
+                                         openers.back(), tok[0]);
+
+                       openers.pop_back();
+                       dest += tok;
+                       continue;
+               }
+               if (tok == "]") {
+                       char opener = openers.empty() ? '(' : openers.back();
+                       if (opener != '[')
+                               log_error("Mismatched brackets in macro argument: %c and %c.\n",
+                                         opener, tok[0]);
+
+                       openers.pop_back();
+                       dest += tok;
+                       continue;
+               }
+               if (tok == "}") {
+                       char opener = openers.empty() ? '(' : openers.back();
+                       if (opener != '{')
+                               log_error("Mismatched brackets in macro argument: %c and %c.\n",
+                                         opener, tok[0]);
+
+                       openers.pop_back();
+                       dest += tok;
+                       continue;
+               }
+
+               if (tok == "," && openers.empty()) {
+                       return false;
+               }
+
+               if (tok == "(" || tok == "[" || tok == "{")
+                       openers.push_back(tok[0]);
 
-static bool try_expand_macro(std::set<std::string> &defines_with_args,
-                            std::map<std::string, std::string> &defines_map,
-                            std::string &tok
-                                   )
+               dest += tok;
+       }
+}
+
+static bool try_expand_macro(define_map_t &defines, std::string &tok)
 {
        if (tok == "`\"") {
                std::string literal("\"");
@@ -229,54 +448,272 @@ static bool try_expand_macro(std::set<std::string> &defines_with_args,
                        if (ntok == "`\"") {
                                insert_input(literal+"\"");
                                return true;
-                       } else if (!try_expand_macro(defines_with_args, defines_map, ntok)) {
+                       } else if (!try_expand_macro(defines, ntok)) {
                                        literal += ntok;
                        }
                }
                return false; // error - unmatched `"
-       } else if (tok.size() > 1 && tok[0] == '`' && defines_map.count(tok.substr(1)) > 0) {
-                       std::string name = tok.substr(1);
-                       // printf("expand: >>%s<< -> >>%s<<\n", name.c_str(), defines_map[name].c_str());
-                       std::string skipped_spaces = skip_spaces();
-                       tok = next_token(false);
-                       if (tok == "(" && defines_with_args.count(name) > 0) {
-                               int level = 1;
-                               std::vector<std::string> args;
-                               args.push_back(std::string());
-                               while (1)
-                               {
-                                       skip_spaces();
-                                       tok = next_token(true);
-                                       if (tok == ")" || tok == "}" || tok == "]")
-                                               level--;
-                                       if (level == 0)
-                                               break;
-                                       if (level == 1 && tok == ",")
-                                               args.push_back(std::string());
-                                       else
-                                               args.back() += tok;
-                                       if (tok == "(" || tok == "{" || tok == "[")
-                                               level++;
-                               }
-                               for (int i = 0; i < GetSize(args); i++)
-                                       defines_map[stringf("macro_%s_arg%d", name.c_str(), i+1)] = args[i];
-                       } else {
-                               insert_input(tok);
-                               insert_input(skipped_spaces);
-                       }
-                       insert_input(defines_map[name]);
-                       return true;
-       } else if (tok == "``") {
+       }
+
+       if (tok == "``") {
                // Swallow `` in macro expansion
                return true;
-       } else return false;
+       }
+
+       if (tok.size() <= 1 || tok[0] != '`')
+               return false;
+
+       // This token looks like a macro name (`foo).
+       std::string macro_name = tok.substr(1);
+       const define_body_t *body = defines.find(tok.substr(1));
+
+       if (! body) {
+               // Apparently not a name we know.
+               return false;
+       }
+
+       std::string name = tok.substr(1);
+       std::string skipped_spaces = skip_spaces();
+       tok = next_token(false);
+       if (tok == "(" && body->has_args) {
+               std::vector<std::string> args;
+               bool done = false;
+               while (!done) {
+                       std::string arg;
+                       done = read_argument(arg);
+                       args.push_back(arg);
+               }
+               for (const auto &pr : body->args.get_vals(name, args)) {
+                       defines.add(pr.first, pr.second);
+               }
+       } else {
+               insert_input(tok);
+               insert_input(skipped_spaces);
+       }
+       insert_input(body->body);
+       return true;
 }
 
-std::string frontend_verilog_preproc(std::istream &f, std::string filename, const std::map<std::string, std::string> &pre_defines_map,
-               dict<std::string, std::pair<std::string, bool>> &global_defines_cache, const std::list<std::string> &include_dirs)
+// Read the arguments for a `define preprocessor directive with formal arguments. This is called
+// just after reading the token containing "(". Returns the number of newlines to emit afterwards to
+// keep line numbers in sync, together with the map from argument name to data (pos and default
+// value).
+static std::pair<int, arg_map_t>
+read_define_args()
 {
-       std::set<std::string> defines_with_args;
-       std::map<std::string, std::string> defines_map(pre_defines_map);
+       // Each argument looks like one of the following:
+       //
+       //     identifier
+       //     identifier = default_text
+       //     identifier =
+       //
+       // The first example is an argument with no default value. The second is an argument whose
+       // default value is default_text. The third is an argument with default value the empty
+       // string.
+
+       int newline_count = 0;
+       arg_map_t args;
+
+       // FSM state.
+       //
+       //   0: At start of identifier
+       //   1: After identifier (stored in arg_name)
+       //   2: After closing paren
+       int state = 0;
+
+       std::string arg_name, default_val;
+
+       skip_spaces();
+       for (;;) {
+               if (state == 2)
+                       // We've read the closing paren.
+                       break;
+
+               std::string tok = next_token();
+
+               // Cope with escaped EOLs
+               if (tok == "\\") {
+                       char ch = next_char();
+                       if (ch == '\n') {
+                               // Eat the \, the \n and any trailing space and keep going.
+                               skip_spaces();
+                               continue;
+                       } else {
+                               // There aren't any other situations where a backslash makes sense.
+                               log_error("Backslash in macro arguments (not at end of line).\n");
+                       }
+               }
+
+               switch (state) {
+               case 0:
+                       // At start of argument. If the token is ')', we've presumably just seen
+                       // something like "`define foo() ...". Set state to 2 to finish. Otherwise,
+                       // the token should be a valid simple identifier, but we'll allow anything
+                       // here.
+                       if (tok == ")") {
+                               state = 2;
+                       } else {
+                               arg_name = tok;
+                               state = 1;
+                       }
+                       skip_spaces();
+                       break;
+
+               case 1:
+                       // After argument. The token should either be an equals sign or a comma or
+                       // closing paren.
+                       if (tok == "=") {
+                               std::string default_val;
+                               //Read an argument into default_val and set state to 2 if we're at
+                               // the end; 0 if we hit a comma.
+                               state = read_argument(default_val) ? 2 : 0;
+                               args.add_arg(arg_name, default_val.c_str());
+                               skip_spaces();
+                               break;
+                       }
+                       if (tok == ",") {
+                               // Take the identifier as an argument with no default value.
+                               args.add_arg(arg_name, nullptr);
+                               state = 0;
+                               skip_spaces();
+                               break;
+                       }
+                       if (tok == ")") {
+                               // As with comma, but set state to 2 (end of args)
+                               args.add_arg(arg_name, nullptr);
+                               state = 2;
+                               skip_spaces();
+                               break;
+                       }
+                       log_error("Trailing contents after identifier in macro argument `%s': "
+                                 "expected '=', ',' or ')'.\n",
+                                 arg_name.c_str());
+
+               default:
+                       // The only FSM states are 0-2 and we dealt with 2 at the start of the loop.
+                       __builtin_unreachable();
+               }
+       }
+
+       return std::make_pair(newline_count, args);
+}
+
+// Read a `define preprocessor directive. This is called just after reading the token containing
+// "`define".
+static void
+read_define(const std::string &filename,
+            define_map_t      &defines_map,
+            define_map_t      &global_defines_cache)
+{
+       std::string name, value;
+       arg_map_t args;
+
+       skip_spaces();
+       name = next_token(true);
+
+       bool here_doc_mode = false;
+       int newline_count = 0;
+
+       // The FSM state starts at 0. If it sees space (or enters here_doc_mode), it assumes this is
+       // a macro without formal arguments and jumps to state 1.
+       //
+       // In state 0, if it sees an opening parenthesis, it assumes this is a macro with formal
+       // arguments. It reads the arguments with read_define_args() and then jumps to state 2.
+       //
+       // In states 1 or 2, the FSM reads tokens to the end of line (or end of here_doc): this is
+       // the body of the macro definition.
+       int state = 0;
+
+       if (skip_spaces() != "")
+               state = 1;
+
+       for (;;) {
+               std::string tok = next_token();
+               if (tok.empty())
+                       break;
+
+               // printf("define-tok: >>%s<<\n", tok != "\n" ? tok.c_str() : "NEWLINE");
+
+               if (tok == "\"\"\"") {
+                       here_doc_mode = !here_doc_mode;
+                       continue;
+               }
+
+               if (state == 0 && tok == "(") {
+                       auto pr = read_define_args();
+                       newline_count += pr.first;
+                       args = pr.second;
+
+                       state = 2;
+                       continue;
+               }
+
+               // This token isn't an opening parenthesis immediately following the macro name, so
+               // it's presumably at or after the start of the macro body. If state isn't already 2
+               // (which would mean we'd parsed an argument list), set it to 1.
+               if (state == 0) {
+                       state = 1;
+               }
+
+               if (tok == "\n") {
+                       if (here_doc_mode) {
+                               value += " ";
+                               newline_count++;
+                       } else {
+                               return_char('\n');
+                               break;
+                       }
+                       continue;
+               }
+
+               if (tok == "\\") {
+                       char ch = next_char();
+                       if (ch == '\n') {
+                               value += " ";
+                               newline_count++;
+                       } else {
+                               value += std::string("\\");
+                               return_char(ch);
+                       }
+                       continue;
+               }
+
+               // Is this token the name of a macro argument? If so, replace it with a magic symbol
+               // that we'll replace with the argument value.
+               int arg_pos;
+               if (args.find(tok, &arg_pos)) {
+                       value += '`' + args.str_token(name, arg_pos);
+                       continue;
+               }
+
+               // This token is nothing special. Insert it verbatim into the macro body.
+               value += tok;
+       }
+
+       // Append some newlines so that we don't mess up line counts in error messages.
+       while (newline_count-- > 0)
+               return_char('\n');
+
+       if (strchr("abcdefghijklmnopqrstuvwxyz_ABCDEFGHIJKLMNOPQRSTUVWXYZ$0123456789", name[0])) {
+               // printf("define: >>%s<< -> >>%s<<\n", name.c_str(), value.c_str());
+               defines_map.add(name, value, (state == 2) ? &args : nullptr);
+               global_defines_cache.add(name, value, (state == 2) ? &args : nullptr);
+       } else {
+               log_file_error(filename, 0, "Invalid name for macro definition: >>%s<<.\n", name.c_str());
+       }
+}
+
+std::string
+frontend_verilog_preproc(std::istream                 &f,
+                         std::string                   filename,
+                         const define_map_t           &pre_defines,
+                         define_map_t                 &global_defines_cache,
+                         const std::list<std::string> &include_dirs)
+{
+       define_map_t defines;
+       defines.merge(pre_defines);
+       defines.merge(global_defines_cache);
+
        std::vector<std::string> filename_stack;
        int ifdef_fail_level = 0;
        bool in_elseif = false;
@@ -287,18 +724,6 @@ std::string frontend_verilog_preproc(std::istream &f, std::string filename, cons
 
        input_file(f, filename);
 
-       defines_map["YOSYS"] = "1";
-       defines_map[formal_mode ? "FORMAL" : "SYNTHESIS"] = "1";
-
-       for (auto &it : pre_defines_map)
-               defines_map[it.first] = it.second;
-
-       for (auto &it : global_defines_cache) {
-               if (it.second.second)
-                       defines_with_args.insert(it.first);
-               defines_map[it.first] = it.second.first;
-       }
-
        while (!input_buffer.empty())
        {
                std::string tok = next_token();
@@ -325,7 +750,7 @@ std::string frontend_verilog_preproc(std::istream &f, std::string filename, cons
                        std::string name = next_token(true);
                        if (ifdef_fail_level == 0)
                                ifdef_fail_level = 1, in_elseif = true;
-                       else if (ifdef_fail_level == 1 && defines_map.count(name) != 0)
+                       else if (ifdef_fail_level == 1 && defines.find(name))
                                ifdef_fail_level = 0, in_elseif = true;
                        continue;
                }
@@ -333,7 +758,7 @@ std::string frontend_verilog_preproc(std::istream &f, std::string filename, cons
                if (tok == "`ifdef") {
                        skip_spaces();
                        std::string name = next_token(true);
-                       if (ifdef_fail_level > 0 || defines_map.count(name) == 0)
+                       if (ifdef_fail_level > 0 || !defines.find(name))
                                ifdef_fail_level++;
                        continue;
                }
@@ -341,7 +766,7 @@ std::string frontend_verilog_preproc(std::istream &f, std::string filename, cons
                if (tok == "`ifndef") {
                        skip_spaces();
                        std::string name = next_token(true);
-                       if (ifdef_fail_level > 0 || defines_map.count(name) != 0)
+                       if (ifdef_fail_level > 0 || defines.find(name))
                                ifdef_fail_level++;
                        continue;
                }
@@ -355,7 +780,7 @@ std::string frontend_verilog_preproc(std::istream &f, std::string filename, cons
                if (tok == "`include") {
                        skip_spaces();
                        std::string fn = next_token(true);
-                       while (try_expand_macro(defines_with_args, defines_map, fn)) {
+                       while (try_expand_macro(defines, fn)) {
                                fn = next_token();
                        }
                        while (1) {
@@ -433,74 +858,7 @@ std::string frontend_verilog_preproc(std::istream &f, std::string filename, cons
                }
 
                if (tok == "`define") {
-                       std::string name, value;
-                       std::map<std::string, int> args;
-                       skip_spaces();
-                       name = next_token(true);
-                       bool here_doc_mode = false;
-                       int newline_count = 0;
-                       int state = 0;
-                       if (skip_spaces() != "")
-                               state = 3;
-                       while (!tok.empty()) {
-                               tok = next_token();
-                               if (tok == "\"\"\"") {
-                                       here_doc_mode = !here_doc_mode;
-                                       continue;
-                               }
-                               if (state == 0 && tok == "(") {
-                                       state = 1;
-                                       skip_spaces();
-                               } else
-                               if (state == 1) {
-                                       if (tok == ")")
-                                               state = 2;
-                                       else if (tok != ",") {
-                                               int arg_idx = args.size()+1;
-                                               args[tok] = arg_idx;
-                                       }
-                                       skip_spaces();
-                               } else {
-                                       if (state != 2)
-                                               state = 3;
-                                       if (tok == "\n") {
-                                               if (here_doc_mode) {
-                                                       value += " ";
-                                                       newline_count++;
-                                               } else {
-                                                       return_char('\n');
-                                                       break;
-                                               }
-                                       } else
-                                       if (tok == "\\") {
-                                               char ch = next_char();
-                                               if (ch == '\n') {
-                                                       value += " ";
-                                                       newline_count++;
-                                               } else {
-                                                       value += std::string("\\");
-                                                       return_char(ch);
-                                               }
-                                       } else
-                                       if (args.count(tok) > 0)
-                                               value += stringf("`macro_%s_arg%d", name.c_str(), args.at(tok));
-                                       else
-                                               value += tok;
-                               }
-                       }
-                       while (newline_count-- > 0)
-                               return_char('\n');
-                       if (strchr("abcdefghijklmnopqrstuvwxyz_ABCDEFGHIJKLMNOPQRSTUVWXYZ$0123456789", name[0])) {
-                               // printf("define: >>%s<< -> >>%s<<\n", name.c_str(), value.c_str());
-                               defines_map[name] = value;
-                               if (state == 2)
-                                       defines_with_args.insert(name);
-                               else
-                                       defines_with_args.erase(name);
-                               global_defines_cache[name] = std::pair<std::string, bool>(value, state == 2);
-                       } else {
-                               log_file_error(filename, 0, "Invalid name for macro definition: >>%s<<.\n", name.c_str());
-                       }
+                       read_define(filename, defines, global_defines_cache);
                        continue;
                }
 
@@ -509,8 +867,7 @@ std::string frontend_verilog_preproc(std::istream &f, std::string filename, cons
                        skip_spaces();
                        name = next_token(true);
                        // printf("undef: >>%s<<\n", name.c_str());
-                       defines_map.erase(name);
-                       defines_with_args.erase(name);
+                       defines.erase(name);
                        global_defines_cache.erase(name);
                        continue;
                }
@@ -525,13 +882,12 @@ std::string frontend_verilog_preproc(std::istream &f, std::string filename, cons
                }
 
                if (tok == "`resetall") {
-                       defines_map.clear();
-                       defines_with_args.clear();
+                       defines.clear();
                        global_defines_cache.clear();
                        continue;
                }
 
-               if (try_expand_macro(defines_with_args, defines_map, tok))
+               if (try_expand_macro(defines, tok))
                        continue;
 
                output_code.push_back(tok);
diff --git a/frontends/verilog/preproc.h b/frontends/verilog/preproc.h
new file mode 100644 (file)
index 0000000..673d633
--- /dev/null
@@ -0,0 +1,77 @@
+/*
+ *  yosys -- Yosys Open SYnthesis Suite
+ *
+ *  Copyright (C) 2012  Clifford Wolf <clifford@clifford.at>
+ *
+ *  Permission to use, copy, modify, and/or distribute this software for any
+ *  purpose with or without fee is hereby granted, provided that the above
+ *  copyright notice and this permission notice appear in all copies.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ *  WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ *  MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ *  ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ *  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ *  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ *  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ *  ---
+ *
+ *  The Verilog preprocessor.
+ *
+ */
+#ifndef VERILOG_PREPROC_H
+#define VERILOG_PREPROC_H
+
+#include "kernel/yosys.h"
+
+#include <iosfwd>
+#include <list>
+#include <memory>
+#include <string>
+
+YOSYS_NAMESPACE_BEGIN
+
+struct define_body_t;
+struct arg_map_t;
+
+struct define_map_t
+{
+       define_map_t();
+       ~ define_map_t();
+
+       // Add a definition, overwriting any existing definition for name.
+       void add(const std::string &name, const std::string &txt, const arg_map_t *args = nullptr);
+
+       // Merge in another map of definitions (which take precedence
+       // over anything currently defined).
+       void merge(const define_map_t &map);
+
+       // Find a definition by name. If no match, returns null.
+       const define_body_t *find(const std::string &name) const;
+
+       // Erase a definition by name (no effect if not defined).
+       void erase(const std::string &name);
+
+       // Clear any existing definitions
+       void clear();
+
+       // Print a list of definitions, using the log function
+       void log() const;
+
+       std::map<std::string, std::unique_ptr<define_body_t>> defines;
+};
+
+
+struct define_map_t;
+
+std::string
+frontend_verilog_preproc(std::istream                 &f,
+                         std::string                   filename,
+                         const define_map_t           &pre_defines,
+                         define_map_t                 &global_defines_cache,
+                         const std::list<std::string> &include_dirs);
+
+YOSYS_NAMESPACE_END
+
+#endif
index f2c1c227f39f5a09e7b54b83bce3672ad00c6a19..a2334e654f61d4122ebfe44667f3284b665d61c8 100644 (file)
@@ -27,6 +27,7 @@
  */
 
 #include "verilog_frontend.h"
+#include "preproc.h"
 #include "kernel/yosys.h"
 #include "libs/sha1/sha1.h"
 #include <stdarg.h>
@@ -253,7 +254,8 @@ struct VerilogFrontend : public Frontend {
                bool flag_defer = false;
                bool flag_noblackbox = false;
                bool flag_nowb = false;
-               std::map<std::string, std::string> defines_map;
+               define_map_t defines_map;
+
                std::list<std::string> include_dirs;
                std::list<std::string> attributes;
 
@@ -369,7 +371,7 @@ struct VerilogFrontend : public Frontend {
                        }
                        if (arg == "-lib") {
                                lib_mode = true;
-                               defines_map["BLACKBOX"] = string();
+                               defines_map.add("BLACKBOX", "");
                                continue;
                        }
                        if (arg == "-nowb") {
@@ -421,7 +423,7 @@ struct VerilogFrontend : public Frontend {
                                        value = name.substr(equal+1);
                                        name = name.substr(0, equal);
                                }
-                               defines_map[name] = value;
+                               defines_map.add(name, value);
                                continue;
                        }
                        if (arg.compare(0, 2, "-D") == 0) {
@@ -430,7 +432,7 @@ struct VerilogFrontend : public Frontend {
                                std::string value;
                                if (equal != std::string::npos)
                                        value = arg.substr(equal+1);
-                               defines_map[name] = value;
+                               defines_map.add(name, value);
                                continue;
                        }
                        if (arg == "-I" && argidx+1 < args.size()) {
@@ -460,7 +462,7 @@ struct VerilogFrontend : public Frontend {
                std::string code_after_preproc;
 
                if (!flag_nopp) {
-                       code_after_preproc = frontend_verilog_preproc(*f, filename, defines_map, design->verilog_defines, include_dirs);
+                       code_after_preproc = frontend_verilog_preproc(*f, filename, defines_map, *design->verilog_defines, include_dirs);
                        if (flag_ppdump)
                                log("-- Verilog code after preprocessor --\n%s-- END OF DUMP --\n", code_after_preproc.c_str());
                        lexin = new std::istringstream(code_after_preproc);
@@ -592,7 +594,7 @@ struct VerilogDefines : public Pass {
                                        value = name.substr(equal+1);
                                        name = name.substr(0, equal);
                                }
-                               design->verilog_defines[name] = std::pair<std::string, bool>(value, false);
+                               design->verilog_defines->add(name, value);
                                continue;
                        }
                        if (arg.compare(0, 2, "-D") == 0) {
@@ -601,27 +603,25 @@ struct VerilogDefines : public Pass {
                                std::string value;
                                if (equal != std::string::npos)
                                        value = arg.substr(equal+1);
-                               design->verilog_defines[name] = std::pair<std::string, bool>(value, false);
+                               design->verilog_defines->add(name, value);
                                continue;
                        }
                        if (arg == "-U" && argidx+1 < args.size()) {
                                std::string name = args[++argidx];
-                               design->verilog_defines.erase(name);
+                               design->verilog_defines->erase(name);
                                continue;
                        }
                        if (arg.compare(0, 2, "-U") == 0) {
                                std::string name = arg.substr(2);
-                               design->verilog_defines.erase(name);
+                               design->verilog_defines->erase(name);
                                continue;
                        }
                        if (arg == "-reset") {
-                               design->verilog_defines.clear();
+                               design->verilog_defines->clear();
                                continue;
                        }
                        if (arg == "-list") {
-                               for (auto &it : design->verilog_defines) {
-                                       log("`define %s%s %s\n", it.first.c_str(), it.second.second ? "()" : "", it.second.first.c_str());
-                               }
+                               design->verilog_defines->log();
                                continue;
                        }
                        break;
index 73ea51e6cbdc892a9eb373ded1555874da8fd3b3..636a4ceca1993770dbc672d3d3a2bbf02e5f5605 100644 (file)
@@ -85,10 +85,6 @@ namespace VERILOG_FRONTEND
        extern std::istream *lexin;
 }
 
-// the pre-processor
-std::string frontend_verilog_preproc(std::istream &f, std::string filename, const std::map<std::string, std::string> &pre_defines_map,
-               dict<std::string, std::pair<std::string, bool>> &global_defines_cache, const std::list<std::string> &include_dirs);
-
 YOSYS_NAMESPACE_END
 
 // the usual bison/flex stuff
index 06181b763a5ba9cfce83f1d0c9975d6acb1d7bcc..79e50cccd57832b2029f1999b2fc47744d0d6db0 100644 (file)
@@ -21,6 +21,7 @@
 #include "kernel/macc.h"
 #include "kernel/celltypes.h"
 #include "frontends/verilog/verilog_frontend.h"
+#include "frontends/verilog/preproc.h"
 #include "backends/ilang/ilang_backend.h"
 
 #include <string.h>
@@ -379,6 +380,7 @@ void RTLIL::Selection::optimize(RTLIL::Design *design)
 }
 
 RTLIL::Design::Design()
+  : verilog_defines (new define_map_t)
 {
        static unsigned int hashidx_count = 123456789;
        hashidx_count = mkhash_xorshift(hashidx_count);
index 58c5d9674b247c52eb818eafaf57d3fa84753c27..4afe4304f8b21d828e74f915100ada8f471e9a9b 100644 (file)
@@ -952,6 +952,9 @@ struct RTLIL::Monitor
        virtual void notify_blackout(RTLIL::Module*) { }
 };
 
+// Forward declaration; defined in preproc.h.
+struct define_map_t;
+
 struct RTLIL::Design
 {
        unsigned int hashidx_;
@@ -963,7 +966,7 @@ struct RTLIL::Design
        int refcount_modules_;
        dict<RTLIL::IdString, RTLIL::Module*> modules_;
        std::vector<AST::AstNode*> verilog_packages, verilog_globals;
-       dict<std::string, std::pair<std::string, bool>> verilog_defines;
+       std::unique_ptr<define_map_t> verilog_defines;
 
        std::vector<RTLIL::Selection> selection_stack;
        dict<RTLIL::IdString, RTLIL::Selection> selection_vars;
index 172addcc13097116590c83ca35b464d983a1d05b..fab23fc1d6e6d22783e4d52e23eab45261a02138 100644 (file)
@@ -18,6 +18,7 @@
  */
 
 #include "kernel/yosys.h"
+#include "frontends/verilog/preproc.h"
 #include "frontends/ast/ast.h"
 
 YOSYS_NAMESPACE_BEGIN
@@ -346,7 +347,7 @@ struct DesignPass : public Pass {
                                delete node;
                        design->verilog_globals.clear();
 
-                       design->verilog_defines.clear();
+                       design->verilog_defines->clear();
                }
 
                if (!load_name.empty() || pop_mode)
diff --git a/tests/various/sv_defines.ys b/tests/various/sv_defines.ys
new file mode 100644 (file)
index 0000000..8e70ee0
--- /dev/null
@@ -0,0 +1,33 @@
+# Check that basic macro expansions do what you'd expect
+
+read_verilog <<EOT
+`define empty_arglist()                      123
+`define one_arg(x)                           123+x
+`define opt_arg(x = 1)                       123+x
+`define two_args(x, y = (1+23))              x+y
+`define nested_comma(x = {31'b0, 1'b1}, y=3) x+y
+
+module top;
+  localparam a = `empty_arglist();
+  localparam b = `one_arg(10);
+  localparam c = `opt_arg(10);
+  localparam d = `opt_arg();
+  localparam e = `two_args(1,2);
+  localparam f = `two_args(1);
+  localparam g = `nested_comma(1, 2);
+  localparam h = `nested_comma({31'b0, (1'b0)});
+  localparam i = `nested_comma(, 1);
+
+  generate
+    if (a != 123) $error("a bad");
+    if (b != 133) $error("b bad");
+    if (c != 133) $error("c bad");
+    if (d != 124) $error("d bad");
+    if (e != 3)   $error("e bad");
+    if (f != 25)  $error("f bad");
+    if (g != 3)   $error("g bad");
+    if (h != 3)   $error("h bad");
+    if (i != 2)   $error("i bad");
+  endgenerate
+endmodule
+EOT
diff --git a/tests/various/sv_defines_dup.ys b/tests/various/sv_defines_dup.ys
new file mode 100644 (file)
index 0000000..38418ba
--- /dev/null
@@ -0,0 +1,5 @@
+# Check for duplicate arguments
+logger -expect error "Duplicate macro arguments with name `x'" 1
+read_verilog <<EOT
+`define duplicate_arg(x, x)
+EOT
diff --git a/tests/various/sv_defines_mismatch.ys b/tests/various/sv_defines_mismatch.ys
new file mode 100644 (file)
index 0000000..ab6e899
--- /dev/null
@@ -0,0 +1,5 @@
+# Check that we spot mismatched brackets
+logger -expect error "Mismatched brackets in macro argument: \[ and }." 1
+read_verilog <<EOT
+`define foo(x=[1,2})
+EOT
diff --git a/tests/various/sv_defines_too_few.ys b/tests/various/sv_defines_too_few.ys
new file mode 100644 (file)
index 0000000..2958848
--- /dev/null
@@ -0,0 +1,7 @@
+# Check that we don't allow passing too few arguments (and, while we're at it, check that passing "no"
+# arguments actually passes 1 empty argument).
+logger -expect error "Cannot expand macro `foo by giving only 1 argument \(argument 2 has no default\)." 1
+read_verilog <<EOT
+`define foo(x=1, y)
+`foo()
+EOT