Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add API to overwrite existing pass from plugin #4174

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Conversation

clairexen
Copy link
Member

No description provided.

@povik
Copy link
Member

povik commented Feb 5, 2024

Code looks good but the Python binding interacts badly with it:

[ 99%] Building kernel/python_wrappers.o
kernel/python_wrappers.cc:417:16: error: 'replace_existing_pass' marked 'override' but does not override any member functions
                virtual bool replace_existing_pass() override;
                             ^
kernel/python_wrappers.cc:417:16: warning: 'YOSYS_PYTHON::Pass::replace_existing_pass' hides overloaded virtual function [-Woverloaded-virtual]
./kernel/register.h:73:15: note: hidden overloaded virtual function 'Yosys::Pass::replace_existing_pass' declared here: different qualifiers ('const' vs unqualified)
        virtual bool replace_existing_pass() const { return false; }
                     ^
1 warning and 1 error generated.

@povik
Copy link
Member

povik commented Feb 5, 2024

It's not pretty but I pushed a fix to the binding generator to handle this.

The below loads:

import libyosys as ys

class StatPass(ys.Pass):
    def __init__(self):
        super().__init__("stat", "")

    def py_replace_existing_pass(self):
        return True

    def py_execute(self, args, design):
        pass

p = StatPass()

@mmicko
Copy link
Member

mmicko commented Feb 5, 2024

Wrote directly to @povik but just to confirm that I have compared generated file before and after, and not just that it fixes this one problematic line but also improves const correctness all over the place in python API wrapper for other calls.

@clairexen clairexen merged commit 1b73b5b into master Feb 5, 2024
34 checks passed
@clairexen clairexen deleted the claire/overwrite branch February 5, 2024 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants