Merge2

[akashlevy]

If your work is part of a larger effort, please discuss your general plans on Discourse first to align your vision with maintainers.

What are the reasons/motivation for this change?

Explain how this is achieved.

Make sure your change comes with tests. If not possible, share how a reviewer might evaluate it.

These template prompts can be deleted when you're done responding to them.

---

Important

Update Yosys to version 0.62 and add process reuse feature in abc.cc for improved performance.

• Version Updates:
  • Update version to 0.62 in Makefile and conf.py.
  • Update CHANGELOG for version 0.61 .. 0.62 with new features and commands.
• ABC Process Reuse:
  • Add REUSE_YOSYS_ABC_PROCESSES macro in abc.cc to enable process reuse when YOSYS_ENABLE_READLINE is defined.
  • Modify RunAbcState::run() to reuse processes from process_pool if is_yosys_abc() returns true.
  • Implement spawn_abc() function to handle process creation with pipes for communication.
• Miscellaneous:
  • Update bumpversion target in Makefile to use new commit hash.
  • Minor logging and error handling improvements in abc.cc.

^{This description was created by for 241852e. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 241852e in 19 seconds. Click for details.

• Reviewed 236 lines of code in 4 files
• Skipped 0 files when reviewing.
• Skipped posting 0 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_jY2hx7sHLirMGXe7

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR merges upstream changes from version 0.61 to 0.62, including version bumps and refactoring of ABC process management in passes/techmap/abc.cc.

Key Changes:

• Version bumped from 0.61 to 0.62 across CHANGELOG, Makefile, and documentation
• Added is_yosys_abc() helper method to AbcConfig struct
• Refactored ABC process reuse logic with new REUSE_YOSYS_ABC_PROCESSES macro
• Restructured control flow in RunAbcState::run() using do-while pattern

Critical Issues:

• Line 1423 in passes/techmap/abc.cc contains unreachable code - log message will never execute
• Line 1415 has inconsistent string handling (missing .c_str() calls)

The refactoring changes the conditional compilation structure and control flow significantly, which increases complexity without clear documentation of the changes.

Confidence Score: 1/5

• This PR contains critical logic errors that will cause incorrect runtime behavior
• The unreachable code on line 1423 indicates a refactoring error where control flow logic was not properly updated. This results in incorrect log output and potentially incorrect state management (did_run flag not set properly).
• passes/techmap/abc.cc requires immediate attention - lines 1415 and 1423 must be fixed before merge

Important Files Changed

Filename	Overview
passes/techmap/abc.cc	Refactored ABC process handling with new conditional compilation and control flow, but contains critical unreachable code bug on line 1423

Sequence Diagram

sequenceDiagram
    participant Module as RTLIL::Module
    participant RunAbcState as RunAbcState::run()
    participant ProcessPool as ConcurrentStack<AbcProcess>
    participant ABC as ABC Executable
    participant Filter as abc_output_filter

    Module->>RunAbcState: prepare_module()
    RunAbcState->>RunAbcState: Write input.blif
    RunAbcState->>RunAbcState: Generate abc.script
    
    alt count_output == 0
        RunAbcState->>Module: Early return (nothing to map)
    else count_output > 0
        RunAbcState->>Filter: Create output filter
        
        alt YOSYS_LINK_ABC defined
            RunAbcState->>ABC: Call abc::Abc_RealMain()
            ABC->>Filter: Process output
            Filter->>RunAbcState: Filtered logs
        else REUSE_YOSYS_ABC_PROCESSES && is_yosys_abc()
            RunAbcState->>ProcessPool: try_pop_back()
            alt Process available
                ProcessPool->>RunAbcState: Reuse existing process
            else No process available
                RunAbcState->>ABC: spawn_abc()
                ABC->>RunAbcState: New process
            end
            RunAbcState->>ABC: Write commands to pipe
            ABC->>Filter: Read until "YOSYS_ABC_DONE"
            Filter->>RunAbcState: Filtered logs
            alt Success (ret == 0)
                RunAbcState->>ProcessPool: Return process to pool
            end
        else External ABC or fallback
            RunAbcState->>ABC: run_command()
            ABC->>Filter: Output via stdout/stderr
            Filter->>RunAbcState: Filtered logs
        end
        
        alt ret != 0
            RunAbcState->>Module: log_error (script failed)
        else ret == 0
            Note over RunAbcState: BUG: Line 1423 unreachable<br/>did_run never set to true
        end
    end

Loading

[greptile-apps]

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

This log message is unreachable - it's after a return statement on line 1421 and will never execute. Originally this message appeared when count_output == 0, but the refactoring moved the early return to line 1336-1338.

Suggested change

	logs.log("Don't call ABC as there is nothing to map.\n");
	did_run = true;

Prompt To Fix With AI

This is a comment left during a code review.
Path: passes/techmap/abc.cc
Line: 1423:1423

Comment:
This log message is unreachable - it's after a return statement on line 1421 and will never execute. Originally this message appeared when `count_output == 0`, but the refactoring moved the early return to line 1336-1338.

```suggestion
	did_run = true;
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Missing .c_str() on config.exe_file - the original code used config.exe_file.c_str() but this was changed to just config.exe_file. While this may work due to implicit conversion, it's inconsistent with the original code and could cause issues.

Suggested change

	std::string cmd = stringf("\"%s\" -s -f %s 2>&1", config.exe_file, tmp_script_name);
	std::string cmd = stringf("\"%s\" -s -f %s 2>&1", config.exe_file.c_str(), tmp_script_name.c_str());

Prompt To Fix With AI

This is a comment left during a code review.
Path: passes/techmap/abc.cc
Line: 1415:1415

Comment:
Missing `.c_str()` on `config.exe_file` - the original code used `config.exe_file.c_str()` but this was changed to just `config.exe_file`. While this may work due to implicit conversion, it's inconsistent with the original code and could cause issues.

```suggestion
		std::string cmd = stringf("\"%s\" -s -f %s 2>&1", config.exe_file.c_str(), tmp_script_name.c_str());
```

How can I resolve this? If you propose a fix, please make it concise.