himbaechel: add uarch specific options parsing (#1582)

* himbaechel: add uarch specific options parsing

* fix tests

* add reference to additional help

* review comments addressed

* cleanup and unify other uarch

* Adressed PR comments

docs/himbaechel.md

@@ -11,6 +11,8 @@ Python scripting is defined that allows the user to describe a semi-flattened ro
 
 Most of what's written in the [viaduct docs](./viaduct.md) also applies to bootstrapping a Himbächel arch - this also provides a migration path for an existing Viaduct architecture. Just replace `viaduct` with `himbaechel` and `ViaductAPI` with `HimbaechelAPI` - the set of validity checking and custom flow "hooks" that you have access to is designed to be otherwise as close as possible.
 
+Additionally Himbächel API defines `getUArchOptions` enabling specifying additional command line parameters for given architecture only.
+
 However, the key difference is that you will need to generate a "binary blob" chip database. `himbaechel_dbgen/bba.py` provides a framework for this. The typical steps for using this API would be as follows:
  - Create a `Chip` instance
  - For each unique "tile type" in the design (e.g. logic, BRAM, IO - in some cases multiple variants of these may be multiple tile types):

himbaechel/arch.cc

@@ -47,7 +47,8 @@ Arch::Arch(ArchArgs args) : args(args)
     }
     log_info("Using uarch '%s' for device '%s'\n", arch->name.c_str(), args.device.c_str());
     this->args.uarch = arch->name;
-    uarch = arch->create(args.device, args.options);
+    uarch = arch->create(args.device);
+    parse_vopt();
     // Load uarch
     uarch->init_database(this);
     if (!chip_info)
@@ -56,6 +57,64 @@ Arch::Arch(ArchArgs args) : args(args)
     init_tiles();
 }
 
+static void print_vopt_help(const po::options_description &vopt_desc)
+{
+    std::cerr << "Allowed --vopt options:\n";
+    size_t maxlen = 0;
+    std::vector<std::pair<std::string, std::string>> lines;
+
+    for (const auto &opt : vopt_desc.options()) {
+        std::string name = opt->canonical_display_name(1);
+        if (name.rfind("--", 0) == 0)
+            name.erase(0, 2);
+
+        bool takes_value = opt->semantic() && opt->semantic()->max_tokens() > 0;
+        std::string text = takes_value ? "--vopt " + name + "=<arg>" : "--vopt " + name;
+
+        maxlen = std::max(maxlen, text.size());
+        lines.emplace_back(std::move(text), opt->description());
+    }
+
+    for (auto &[text, desc] : lines)
+        std::cerr << "  " << std::left << std::setw(static_cast<int>(maxlen) + 2) << text << desc << "\n";
+}
+
+void Arch::parse_vopt()
+{
+    namespace po = boost::program_options;
+    auto vopt_desc = uarch->getUArchOptions();
+    vopt_desc.add_options()("help,h", "show help");
+
+    std::vector<const char *> argv;
+    for (auto &a : args.vopts)
+        argv.push_back(a.c_str());
+
+    try {
+        po::parsed_options parsed =
+                po::command_line_parser((int)argv.size(), argv.data())
+                        .style(po::command_line_style::default_style ^ po::command_line_style::allow_guessing)
+                        .options(vopt_desc)
+                        .run();
+        po::store(parsed, args.options);
+        po::notify(args.options);
+    } catch (const po::unknown_option &e) {
+        std::string option_name = e.get_option_name();
+        if (!option_name.empty() && option_name[0] == '-') {
+            size_t start = option_name.find_first_not_of('-');
+            option_name = option_name.substr(start);
+        }
+        std::cerr << "Error: unrecognized --vopt option: " << option_name << std::endl;
+        exit(0);
+    } catch (std::exception &e) {
+        std::cout << e.what() << "\n";
+        exit(0);
+    }
+    if (args.options.count("help")) {
+        print_vopt_help(vopt_desc);
+        exit(0);
+    }
+}
+
 void Arch::load_chipdb(const std::string &path)
 {
     std::string db_path;

himbaechel/arch.h

@@ -427,7 +427,8 @@ struct ArchArgs
     std::string uarch;
     std::string chipdb_override;
     std::string device;
-    dict<std::string, std::string> options;
+    std::vector<std::string> vopts;
+    po::variables_map options;
 };
 
 typedef TileObjRange<BelId, BelDataPOD, &TileTypePOD::bels> BelRange;
@@ -476,6 +477,7 @@ struct Arch : BaseArch<ArchRanges>
     void set_package(const std::string &package);
 
     void late_init();
+    void parse_vopt();
 
     // Database references
     boost::iostreams::mapped_file_source blob_file;

himbaechel/himbaechel_api.h

@@ -20,6 +20,7 @@
 #ifndef HIMBAECHEL_API_H
 #define HIMBAECHEL_API_H
 
+#include <boost/program_options.hpp>
 #include "nextpnr_namespaces.h"
 #include "nextpnr_types.h"
 
@@ -55,6 +56,8 @@ struct Context;
 
 struct PlacerHeapCfg;
 
+namespace po = boost::program_options;
+
 struct HimbaechelAPI
 {
     // Architecture specific context initialization
@@ -64,6 +67,8 @@ struct HimbaechelAPI
     // If constids are being used, this is used to set them up early
     // then it is responsible for loading the db blob with arch->load_chipdb()
     virtual void init_database(Arch *arch) = 0;
+    // Return uarch specific options description
+    virtual po::options_description getUArchOptions() = 0;
     Context *ctx;
     bool with_gui = false;
 
@@ -148,8 +153,7 @@ struct HimbaechelArch
     HimbaechelArch(const std::string &name);
     ~HimbaechelArch() {};
     virtual bool match_device(const std::string &device) = 0;
-    virtual std::unique_ptr<HimbaechelAPI> create(const std::string &device,
+    virtual std::unique_ptr<HimbaechelAPI> create(const std::string &device) = 0;
-                                                  const dict<std::string, std::string> &args) = 0;
 
     static std::string list();
     static HimbaechelArch *find_match(const std::string &device);

himbaechel/main.cc

@@ -51,7 +51,8 @@ po::options_description HimbaechelCommandHandler::getArchOptions()
     specific.add_options()("device", po::value<std::string>(), "name of device to use");
     specific.add_options()("chipdb", po::value<std::string>(), "override path to chip database file");
     specific.add_options()("list-uarch", "list included uarches");
-    specific.add_options()("vopt,o", po::value<std::vector<std::string>>(), "options to pass to the himbächel uarch");
+    specific.add_options()("vopt,o", po::value<std::vector<std::string>>(),
+                           "options to pass to the himbächel uarch (use help as argument to get more info)");
 
     return specific;
 }
@@ -83,13 +84,9 @@ std::unique_ptr<Context> HimbaechelCommandHandler::createContext(dict<std::strin
 
     if (vm.count("vopt")) {
         std::vector<std::string> options = vm["vopt"].as<std::vector<std::string>>();
-        for (const auto &opt : options) {
+        chipArgs.vopts.push_back("vopt");
-            size_t epos = opt.find('=');
+        for (const auto &opt : options)
-            if (epos == std::string::npos)
+            chipArgs.vopts.push_back("--" + opt);
-                chipArgs.options[opt] = "";
-            else
-                chipArgs.options[opt.substr(0, epos)] = opt.substr(epos + 1);
-        }
     }
     auto ctx = std::unique_ptr<Context>(new Context(chipArgs));
     if (vm.count("gui"))

himbaechel/uarch/example/example.cc

@@ -40,6 +40,13 @@ struct ExampleImpl : HimbaechelAPI
     static constexpr int K = 4;
 
     ~ExampleImpl() {};
+
+    po::options_description getUArchOptions()
+    {
+        po::options_description specific("Example specific options");
+        return specific;
+    }
+
     void init_database(Arch *arch) override
     {
         init_uarch_constids(arch);
@@ -329,8 +336,7 @@ struct ExampleArch : HimbaechelArch
 {
     ExampleArch() : HimbaechelArch("example") {};
     bool match_device(const std::string &device) override { return device == "EXAMPLE"; }
-    std::unique_ptr<HimbaechelAPI> create(const std::string &device,
+    std::unique_ptr<HimbaechelAPI> create(const std::string &device) override
-                                          const dict<std::string, std::string> &args) override
     {
         return std::make_unique<ExampleImpl>();
     }

himbaechel/uarch/gatemate/gatemate.cc

@@ -32,6 +32,20 @@ NEXTPNR_NAMESPACE_BEGIN
 
 GateMateImpl::~GateMateImpl() {};
 
+po::options_description GateMateImpl::getUArchOptions()
+{
+    po::options_description specific("GateMate specific options");
+    specific.add_options()("out", po::value<std::string>(), "textual configuration bitstream output file");
+    specific.add_options()("ccf", po::value<std::string>(), "name of constraints file");
+    specific.add_options()("allow-unconstrained", "allow unconstrained IOs");
+    specific.add_options()("fpga_mode", po::value<std::string>(), "operation mode (1:lowpower, 2:economy, 3:speed)");
+    specific.add_options()("time_mode", po::value<std::string>(), "timing mode (1:best, 2:typical, 3:worst)");
+    specific.add_options()("strategy", po::value<std::string>(),
+                           "multi-die clock placement strategy (mirror, full or clk1)");
+    specific.add_options()("force_die", po::value<std::string>(), "force specific die (example 1A,1B...)");
+    return specific;
+}
+
 static int parse_mode(const std::string &val, const std::map<std::string, int> &map, const char *error_msg)
 {
     try {
@@ -60,10 +74,10 @@ void GateMateImpl::init_database(Arch *arch)
     static const std::map<std::string, int> timing_map = {{"best", 1}, {"typical", 2}, {"worst", 3}};
 
     if (args.options.count("fpga_mode"))
-        fpga_mode = parse_mode(args.options.at("fpga_mode"), fpga_map,
+        fpga_mode = parse_mode(args.options["fpga_mode"].as<std::string>(), fpga_map,
                                "operation mode valid values are {1:lowpower, 2:economy, 3:speed}");
     if (args.options.count("time_mode"))
-        timing_mode = parse_mode(args.options.at("time_mode"), timing_map,
+        timing_mode = parse_mode(args.options["time_mode"].as<std::string>(), timing_map,
                                  "timing mode valid values are {1:best, 2:typical, 3:worst}");
 
     std::string speed_grade = "";
@@ -157,7 +171,7 @@ void GateMateImpl::init(Context *ctx)
     const ArchArgs &args = ctx->args;
     std::string die_name;
     if (args.options.count("force_die"))
-        die_name = args.options.at("force_die");
+        die_name = args.options["force_die"].as<std::string>();
     bool found = false;
     int index = 0;
     for (auto &die : extra->dies) {
@@ -437,7 +451,7 @@ void GateMateImpl::postRoute()
 
     const ArchArgs &args = ctx->args;
     if (args.options.count("out")) {
-        write_bitstream(args.device, args.options.at("out"));
+        write_bitstream(args.device, args.options["out"].as<std::string>());
     }
 }
 
@@ -606,8 +620,7 @@ struct GateMateArch : HimbaechelArch
     {
         return device.size() > 6 && device.substr(0, 6) == "CCGM1A";
     }
-    std::unique_ptr<HimbaechelAPI> create(const std::string &device,
+    std::unique_ptr<HimbaechelAPI> create(const std::string &device) override
-                                          const dict<std::string, std::string> &args) override
     {
         return std::make_unique<GateMateImpl>();
     }

himbaechel/uarch/gatemate/gatemate.h

@@ -40,6 +40,7 @@ enum MultiDieStrategy
 struct GateMateImpl : HimbaechelAPI
 {
     ~GateMateImpl();
+    po::options_description getUArchOptions() override;
     void init_database(Arch *arch) override;
 
     void init(Context *ctx) override;

himbaechel/uarch/gatemate/pack.cc

@@ -479,11 +479,11 @@ void GateMateImpl::pack()
 {
     const ArchArgs &args = ctx->args;
     if (args.options.count("ccf")) {
-        parse_ccf(args.options.at("ccf"));
+        parse_ccf(args.options["ccf"].as<std::string>());
     }
 
     if (args.options.count("strategy")) {
-        std::string val = args.options.at("strategy");
+        std::string val = args.options["strategy"].as<std::string>();
         if (val == "mirror") {
             strategy = MultiDieStrategy::CLOCK_MIRROR;
             log_info("Multidie mode: CLOCK MIRROR\n");

himbaechel/uarch/gatemate/tests/testing.cc

@@ -30,7 +30,8 @@ void GateMateTest::SetUp()
 {
     init_share_dirname();
     chipArgs.device = "CCGM1A1";
-    chipArgs.options.emplace("allow-unconstrained", "");
+    chipArgs.vopts.push_back("vopt");
+    chipArgs.vopts.push_back("--allow-unconstrained");
     ctx = new Context(chipArgs);
     ctx->uarch->init(ctx);
     ctx->late_init();

himbaechel/uarch/gowin/gowin.cc

@@ -23,6 +23,7 @@ struct GowinImpl : HimbaechelAPI
 {
 
     ~GowinImpl() {};
+    po::options_description getUArchOptions() override;
     void init_database(Arch *arch) override;
     void init(Context *ctx) override;
 
@@ -110,20 +111,30 @@ struct GowinArch : HimbaechelArch
 
     bool match_device(const std::string &device) override { return device.size() > 2 && device.substr(0, 2) == "GW"; }
 
-    std::unique_ptr<HimbaechelAPI> create(const std::string &device,
+    std::unique_ptr<HimbaechelAPI> create(const std::string &device) override { return std::make_unique<GowinImpl>(); }
-                                          const dict<std::string, std::string> &args) override
-    {
-        return std::make_unique<GowinImpl>();
-    }
 } gowinArch;
 
+po::options_description GowinImpl::getUArchOptions()
+{
+    po::options_description specific("Gowin specific options");
+    specific.add_options()("family", po::value<std::string>(), "GOWIN chip family");
+    specific.add_options()("cst", po::value<std::string>(), "name of constraints file");
+    specific.add_options()("ireg_in_iob", "place input registers in IOB");
+    specific.add_options()("oreg_in_iob", "place output registers in IOB");
+    specific.add_options()("ioreg_in_iob", "place I/O registers in IOB");
+    specific.add_options()("disable_gp_clock_routing", "disable clock network routing from GP pins");
+    specific.add_options()("sspi_as_gpio", "use SSPI pins as GPIO");
+    specific.add_options()("i2c_as_gpio", "use I2C pins as GPIO");
+    return specific;
+}
+
 void GowinImpl::init_database(Arch *arch)
 {
     init_uarch_constids(arch);
     const ArchArgs &args = arch->args;
     std::string family;
     if (args.options.count("family")) {
-        family = args.options.at("family");
+        family = args.options["family"].as<std::string>();
     } else {
         bool GW2 = args.device.rfind("GW2A", 0) == 0;
         if (GW2) {
@@ -203,7 +214,7 @@ void GowinImpl::init(Context *ctx)
 
     // constraints
     if (args.options.count("cst")) {
-        ctx->settings[ctx->id("cst.filename")] = args.options.at("cst");
+        ctx->settings[ctx->id("cst.filename")] = args.options["cst"].as<std::string>();
     }
 
     // place registers in IO blocks

himbaechel/uarch/ng-ultra/ng_ultra.cc

@@ -43,6 +43,18 @@ NEXTPNR_NAMESPACE_BEGIN
 
 NgUltraImpl::~NgUltraImpl() {};
 
+po::options_description NgUltraImpl::getUArchOptions()
+{
+    po::options_description specific("NG-Ultra specific options");
+    specific.add_options()("bit", po::value<std::string>(), "textual configuration bitstream output file");
+    specific.add_options()("csv", po::value<std::string>(), "name of constraints file");
+    specific.add_options()("no-xlut", "disable XLUT optimisations");
+    specific.add_options()("no-lut-chains", "disable LUT chains optimisations");
+    specific.add_options()("no-dff-chains", "disable DFF chains optimisations");
+    specific.add_options()("no-csc-insertion", "disable CSC insertion");
+    return specific;
+}
+
 void NgUltraImpl::init_database(Arch *arch)
 {
     init_uarch_constids(arch);
@@ -432,7 +444,7 @@ void NgUltraImpl::postRoute()
     print_utilisation(ctx);
     const ArchArgs &args = ctx->args;
     if (args.options.count("bit")) {
-        write_bitstream_json(args.options.at("bit"));
+        write_bitstream_json(args.options["bit"].as<std::string>());
     }
 }
 
@@ -1060,8 +1072,7 @@ struct NgUltraArch : HimbaechelArch
 {
     NgUltraArch() : HimbaechelArch("ng-ultra") {};
     bool match_device(const std::string &device) override { return device == "NG-ULTRA"; }
-    std::unique_ptr<HimbaechelAPI> create(const std::string &device,
+    std::unique_ptr<HimbaechelAPI> create(const std::string &device) override
-                                          const dict<std::string, std::string> &args) override
     {
         return std::make_unique<NgUltraImpl>();
     }

himbaechel/uarch/ng-ultra/ng_ultra.h

@@ -40,6 +40,7 @@ NEXTPNR_NAMESPACE_BEGIN
 struct NgUltraImpl : HimbaechelAPI
 {
     ~NgUltraImpl();
+    po::options_description getUArchOptions() override;
     void init_database(Arch *arch) override;
 
     void init(Context *ctx) override;

himbaechel/uarch/ng-ultra/pack.cc

@@ -2180,7 +2180,7 @@ void NgUltraImpl::pack()
 {
     const ArchArgs &args = ctx->args;
     if (args.options.count("csv")) {
-        parse_csv(args.options.at("csv"));
+        parse_csv(args.options["csv"].as<std::string>());
     }
 
     // Setup

himbaechel/uarch/xilinx/pack.cc

@@ -727,7 +727,7 @@ void XilinxImpl::pack()
 {
     const ArchArgs &args = ctx->args;
     if (args.options.count("xdc")) {
-        parse_xdc(args.options.at("xdc"));
+        parse_xdc(args.options["xdc"].as<std::string>());
     }
 
     XC7Packer packer(ctx, this);

himbaechel/uarch/xilinx/xilinx.cc

@@ -39,6 +39,14 @@ NEXTPNR_NAMESPACE_BEGIN
 
 XilinxImpl::~XilinxImpl() {};
 
+po::options_description XilinxImpl::getUArchOptions()
+{
+    po::options_description specific("Xilinx specific options");
+    specific.add_options()("fasm", po::value<std::string>(), "fasm bitstream output file");
+    specific.add_options()("xdc", po::value<std::string>(), "name of constraints file");
+    return specific;
+}
+
 void XilinxImpl::init_database(Arch *arch)
 {
     const ArchArgs &args = arch->args;
@@ -299,7 +307,7 @@ void XilinxImpl::postRoute()
     ctx->assignArchInfo();
     const ArchArgs &args = ctx->args;
     if (args.options.count("fasm")) {
-        write_fasm(args.options.at("fasm"));
+        write_fasm(args.options["fasm"].as<std::string>());
     }
 }
 
@@ -576,11 +584,7 @@ struct XilinxArch : HimbaechelArch
 {
     XilinxArch() : HimbaechelArch("xilinx") {};
     bool match_device(const std::string &device) override { return device.size() > 3 && device.substr(0, 3) == "xc7"; }
-    std::unique_ptr<HimbaechelAPI> create(const std::string &device,
+    std::unique_ptr<HimbaechelAPI> create(const std::string &device) override { return std::make_unique<XilinxImpl>(); }
-                                          const dict<std::string, std::string> &args) override
-    {
-        return std::make_unique<XilinxImpl>();
-    }
 } xilinxArch;
 } // namespace
 

himbaechel/uarch/xilinx/xilinx.h

@@ -109,6 +109,7 @@ struct XilinxImpl : HimbaechelAPI
     };
 
     ~XilinxImpl();
+    po::options_description getUArchOptions() override;
     void init_database(Arch *arch) override;
 
     void init(Context *ctx) override;