Avoid name and keyword collisions in generated verilog.

Add a name uniquer on ModuleBuilder to prevent accidental collisions of identifiers in verilog modules (regs, wires, etc).

Other fixes, cleanups:

* xls::verilog::SanitizeIdentifier renamed to SanitizeVerilogIdentifier to distinguish it from other uses of sanitize.

* SanitizeVerilogIdentifier now sanitizes keywords.

* Raise an error if port names collide rather than silently renaming because port names are part of the interface and should not be changed.

* Don't call SanitizeVerilogIdentifer during block creation. We have a separate pass which handes that (NameLeagalizationPass).

Fixes #2209.
Fixes #805

PiperOrigin-RevId: 761267477

Author: meheffernan

xls/codegen/BUILD

@@ -410,6 +410,7 @@ cc_library(
         "//xls/ir:bits_ops",
         "//xls/ir:format_preference",
         "//xls/ir:format_strings",
+        "//xls/ir:name_uniquer",
         "//xls/ir:op",
         "//xls/ir:source_location",
         "//xls/ir:type",
@@ -521,7 +522,6 @@ cc_library(
         ":concurrent_stage_groups",
         ":conversion_utils",
         ":mark_channel_fifos_pass",
-        "//xls/codegen/vast",
         "//xls/common:casts",
         "//xls/common/logging:log_lines",
         "//xls/common/status:ret_check",
@@ -696,12 +696,12 @@ cc_library(
     hdrs = ["name_legalization_pass.h"],
     deps = [
         ":codegen_pass",
+        "//xls/codegen/vast:verilog_keywords",
         "//xls/common/status:ret_check",
         "//xls/common/status:status_macros",
         "//xls/ir",
         "//xls/ir:register",
         "//xls/passes:pass_base",
-        "@com_google_absl//absl/base:no_destructor",
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
@@ -718,6 +718,7 @@ cc_library(
         ":codegen_pass",
         ":module_signature",
         ":module_signature_cc_proto",
+        "//xls/codegen/vast",
         "//xls/common:casts",
         "//xls/common/logging:log_lines",
         "//xls/common/status:ret_check",

xls/codegen/block_conversion.cc

@@ -44,7 +44,6 @@
 #include "xls/codegen/conversion_utils.h"
 #include "xls/codegen/mark_channel_fifos_pass.h"
 #include "xls/codegen/proc_block_conversion.h"
-#include "xls/codegen/vast/vast.h"
 #include "xls/common/logging/log_lines.h"
 #include "xls/common/status/ret_check.h"
 #include "xls/common/status/status_macros.h"
@@ -649,21 +648,17 @@ absl::StatusOr<CodegenContext> PackageToPipelinedBlocks(
 
   // Make `unit` optional because we haven't created the top block yet. We will
   // create it on the first iteration and emplace `unit`.
-  std::string module_name(
-      SanitizeIdentifier(options.module_name().value_or(top->name())));
+  std::string module_name(options.module_name().value_or(top->name()));
   Block* top_block =
       package->AddBlock(std::make_unique<Block>(module_name, package));
   top_block->SetFunctionBaseProvenance(top);
 
   // We use a uniquer here because the top block name comes from the codegen
   // option's `module_name` field (if set). A non-top proc could have the same
   // name, so the name uniquer will ensure that the sub-block gets a suffix if
-  // needed. Note that the NameUniquer's sanitize performs a different function
-  // from `SanitizeIdentifier()`, which is used to ensure that identifiers are
-  // OK for RTL.
+  // needed.
   NameUniquer block_name_uniquer("__");
-  XLS_RET_CHECK_EQ(block_name_uniquer.GetSanitizedUniqueName(module_name),
-                   module_name);
+  block_name_uniquer.GetSanitizedUniqueName(module_name);
   CodegenContext context(top_block);
 
   // Run codegen passes as appropriate
@@ -682,8 +677,8 @@ absl::StatusOr<CodegenContext> PackageToPipelinedBlocks(
         "Missing schedule for functionbase `%s`", fb->name());
 
     const PipelineSchedule& schedule = schedules.at(fb);
-    std::string sub_block_name = block_name_uniquer.GetSanitizedUniqueName(
-        SanitizeIdentifier(fb->name()));
+    std::string sub_block_name =
+        block_name_uniquer.GetSanitizedUniqueName(fb->name());
     Block* sub_block;
     if (fb == top) {
       sub_block = top_block;
@@ -737,8 +732,7 @@ absl::StatusOr<CodegenContext> FunctionToCombinationalBlock(
     Function* f, const CodegenOptions& options) {
   XLS_RET_CHECK(!options.valid_control().has_value())
       << "Combinational block generator does not support valid control.";
-  std::string module_name(
-      options.module_name().value_or(SanitizeIdentifier(f->name())));
+  std::string module_name(options.module_name().value_or(f->name()));
   Block* block = f->package()->AddBlock(
       std::make_unique<Block>(module_name, f->package()));
   block->SetFunctionBaseProvenance(f);
@@ -816,9 +810,9 @@ absl::StatusOr<CodegenContext> ProcToCombinationalBlock(
                       })));
   }
 
-  std::string module_name = SanitizeIdentifier(std::string{
-      options.module_name().has_value() ? options.module_name().value()
-                                        : proc->name()});
+  std::string module_name = std::string{options.module_name().has_value()
+                                            ? options.module_name().value()
+                                            : proc->name()};
   Block* block = proc->package()->AddBlock(
       std::make_unique<Block>(module_name, proc->package()));
   block->SetFunctionBaseProvenance(proc);

xls/codegen/block_conversion_pass_pipeline.cc

@@ -47,7 +47,7 @@ absl::StatusOr<CodegenContext> CreateBlocksFor(
   FunctionBase* top = *package->GetTop();
 
   std::string module_name(
-      SanitizeIdentifier(options.module_name().value_or(top->name())));
+      SanitizeVerilogIdentifier(options.module_name().value_or(top->name())));
 
   Block* top_block =
       package->AddBlock(std::make_unique<Block>(module_name, package));
@@ -58,8 +58,8 @@ absl::StatusOr<CodegenContext> CreateBlocksFor(
   // option's `module_name` field (if set). A non-top proc could have the same
   // name, so the name uniquer will ensure that the sub-block gets a suffix if
   // needed. Note that the NameUniquer's sanitize performs a different function
-  // from `SanitizeIdentifier()`, which is used to ensure that identifiers are
-  // OK for RTL.
+  // from `SanitizeVerilogIdentifier()`, which is used to ensure that
+  // identifiers are OK for RTL.
   NameUniquer block_name_uniquer("__");
   XLS_RET_CHECK_EQ(block_name_uniquer.GetSanitizedUniqueName(module_name),
                    module_name);
@@ -77,7 +77,7 @@ absl::StatusOr<CodegenContext> CreateBlocksFor(
     std::string sub_block_name =
         (fb == top) ? module_name
                     : block_name_uniquer.GetSanitizedUniqueName(
-                          SanitizeIdentifier(fb->name()));
+                          SanitizeVerilogIdentifier(fb->name()));
     Block* block = (fb == top) ? top_block
                                : package->AddBlock(std::make_unique<Block>(
                                      sub_block_name, package));

xls/codegen/block_stitching_pass.cc

@@ -603,7 +603,7 @@ absl::StatusOr<bool> BlockStitchingPass::RunInternal(
         "Splitting outputs is not supported by block stitching.");
   }
   std::string top_block_name(options.codegen_options.module_name().value_or(
-      SanitizeIdentifier(context.top_block()->name())));
+      context.top_block()->name()));
 
   XLS_ASSIGN_OR_RETURN(
       Block * top_block,

xls/codegen/conversion_utils.cc

@@ -134,7 +134,7 @@ std::string PipelineSignalName(std::string_view root, int64_t stage) {
   if (!RE2::PartialMatch(root, *kPipelinePrefix, &base)) {
     base = root;
   }
-  return absl::StrFormat("p%d_%s", stage, SanitizeIdentifier(base));
+  return absl::StrFormat("p%d_%s", stage, base);
 }
 
 absl::StatusOr<std::vector<Node*>> MakeInputReadyPortsForOutputChannels(

xls/codegen/ffi_instantiation_pass.cc

@@ -170,7 +170,7 @@ absl::StatusOr<bool> FfiInstantiationPass::RunInternal(
       }
 
       // TODO(hzeller): Better ways to generate a name ?
-      const std::string inst_name = SanitizeIdentifier(
+      const std::string inst_name = SanitizeVerilogIdentifier(
           absl::StrCat(fun->name(), "_", invocation->GetName(), "_inst"));
       XLS_ASSIGN_OR_RETURN(xls::Instantiation * instantiation,
                            block->AddInstantiation(