macro issue

[HungMingWu]

I met an issue, ask for help.
Here is top.v

`define APB_BASE_ADDR       40'hb0000000
module top(input wire clk);
sub s(.clk(clk));
endmodule

sub.v

module sub(input wire clk);
reg [39 :0]  pad_cpu_apb_base;
assign pad_cpu_apb_base = `APB_BASE_ADDR;
endmodule

In verilator, it works.
But in slang, it output

 sub.v:4:27: error: unknown macro or compiler directive '`APB_BASE_ADDR'
assign pad_cpu_apb_base = `APB_BASE_ADDR;
                          ^
sub.v:4:41: error: expected expression
assign pad_cpu_apb_base = `APB_BASE_ADDR;

The root cause comes from

std::shared_ptr<SyntaxTree> SyntaxTree::create(SourceManager& sourceManager,
                                               span<const SourceBuffer> sources, const Bag& options,
                                               bool guess) {
    BumpAllocator alloc;
    Diagnostics diagnostics;
    Preprocessor preprocessor(sourceManager, alloc, diagnostics, options);

    for (auto it = sources.rbegin(); it != sources.rend(); it++)
        preprocessor.pushSource(*it);

    Parser parser(preprocessor, options);

    SyntaxNode* root;
    if (!guess)
        root = &parser.parseCompilationUnit();
    else {
        root = &parser.parseGuess();
        if (!parser.isDone())
            return create(sourceManager, sources, options, false);
    }

    return std::shared_ptr<SyntaxTree>(new SyntaxTree(root, sourceManager, std::move(alloc),
                                                      std::move(diagnostics), parser.getMetadata(),
                                                      options));
}

The macro is defined in top, and not propagate to sub. due to separate compilation.

My naive solution is put Preprocessor in the top level, like this

bool loadAllSources(Compilation& compilation, SourceManager& sourceManager,
                    const std::vector<SourceBuffer>& buffers, const Bag& options, bool singleUnit,
                    bool onlyLint, const std::vector<std::string>& libraryFiles,
                    const std::vector<std::string>& libDirs,
                    const std::vector<std::string>& libExts) {
    // Ignored
    Preprocessor preprocessor(...);
    // Ignored
    if (singleUnit) {
        auto tree = SyntaxTree::fromBuffers(buffers, sourceManager, preprocessor);
        // Ignored
        compilation.addSyntaxTree(tree);
    }
    // Ignored
    // Keep loading new files as long as we are making forward progress.
    flat_hash_set<string_view> nextMissingNames;
    while (true) {
            // Ignored
            if (buffer) {
                auto tree = SyntaxTree::fromBuffer(buffer, sourceManager, preprocessor);
                // Ignored
            }
    }
    // Ignored
} 

The solution would break API, I don't think it is a good method.
Another idea is putting Preprocessor into SourceManager.

class SourceManager {
    // Ignored
   Preprocessor preprocessor;
};

Anyone have good idea?

[MikePopoloski]

Separate files are by default treated as separate compilation units, which means they don't see each other's macro definitions. If you want them to be treated as a single compilation unit (which is what verilator does by default) you can pass the --single-unit command line flag, which internally concatenates all of the files into a single source file.

[HungMingWu]

Hi Mike,
add --single-unit in the above case, it works.
But in my real case, it doesn't work properly.
I will investigate further

[HungMingWu]

After investigation, I think the root cause is identical.
Here is the original flow

bool loadAllSources(Compilation& compilation, SourceManager& sourceManager,
                    const std::vector<SourceBuffer>& buffers, const Bag& options, bool singleUnit,
                    bool onlyLint, const std::vector<std::string>& libraryFiles,
                    const std::vector<std::string>& libDirs,
                    const std::vector<std::string>& libExts) {
             if (singleUnit) {
                   // Ignored
             } else {
                   // Ignored
             }
             // Ignored
             // Keep loading new files as long as we are making forward progress.
            flat_hash_set<string_view> nextMissingNames;
            while (true) {
                for (auto name : missingNames) {
                        // Ignored
                        if (buffer) {
                            auto tree = SyntaxTree::fromBuffer(buffer, sourceManager, options);
                            tree->isLibrary = true;
                            compilation.addSyntaxTree(tree);
                        }
                }
            }
}

In my case, the buffers size is 1. so single unit doesn't take effect.
My observation is

1. Compile a verilog file, it contains a macro A.
2. The Preprocessor is destroyed.
3. Separate compile each missing_name unit, so it cannot find macro A.

[MikePopoloski]

I see, you are using library files loaded automatically on demand. Library files aren't part of the single unit by design, because they're a separate library. If you want them to be part of the single unit you can specify them all together on the command line.

It's strange to want macros to propagate from other source files into library files, but I suppose that's a new feature that code be added.

[HungMingWu]

My case comes from https://github.com/T-head-Semi/openc910/search?q=APB_BASE_ADDR
It use these idiom very much.
I can help implement this issue.
Would you have any concern?

[MikePopoloski]

You can always use the -D command line flag to define macros that apply to all loaded units. If you want it to work with automatically loaded library units you'll have to wait for me to add a new feature to slang to support it -- can you create an issue to track that work?

[by1e11]

Hi, I have a similar issue when using the command file. Typically, we will have a nested ".lst" file set (as vcs) in which the order is important for preprocessor dependencies. But currently, the commands does not seem to honor the order described in the lst files. Do you think it makes more sense to make the driver to parse the command files and commands in exact order that is described by the command files (.lst files)

For example: I have two lst files:

a.lst:
    a1.v
    -f b.lst
    a2.v

b.lst:
    b1.v
    b2.v

./slang -E -f a.lst

Then the order of the source files for the preprocessor should be "a1.v, b1.v, b2.v, a2.v". In other words, in a recursive manner.

Any comment on this scenario?

Thanks.

[MikePopoloski]

I can look into that. Right now command files are processed completely and only then do I go back and load all nested command files, so they're ordered in that sense but not ordered relative to other sources listed in that same command file.

[MikePopoloski]

I fixed this issue in my latest commit.