Skip to content

refactor(esp): split _exit into separate file for QEMU support#10

Merged
MeteorsLiu merged 1 commit intogoplus:mainfrom
luoliwoshang:refactor/split-exit-syscall
Jan 9, 2026
Merged

refactor(esp): split _exit into separate file for QEMU support#10
MeteorsLiu merged 1 commit intogoplus:mainfrom
luoliwoshang:refactor/split-exit-syscall

Conversation

@luoliwoshang
Copy link
Member

Summary

  • Move _exit implementation from syscalls.c to syscalls_exit.c
  • This separation allows easy replacement with semihosting version for QEMU emulator support
  • Enables llgo to compile the semihosting _exit as a separate object file that can override the default implementation

Related

🤖 Generated with Claude Code

Move _exit implementation from syscalls.c to syscalls_exit.c to allow
easy replacement with semihosting version for QEMU emulator support.

This separation enables llgo to compile the semihosting _exit as a
separate object file that can override the default implementation
when building for QEMU targets.

Related to: goplus/llgo#1539

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@gemini-code-assist
Copy link

Summary of Changes

Hello @luoliwoshang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the _exit system call implementation by isolating it into its own source file. This strategic move enhances modularity and significantly improves compatibility with the QEMU emulator by allowing alternative _exit versions, such as semihosting, to be easily integrated and override the default behavior during compilation.

Highlights

  • _exit implementation refactor: The _exit function's implementation has been moved from syscalls.c to a new, dedicated file named syscalls_exit.c.
  • Enhanced QEMU emulator support: This separation allows for the easy replacement of the default _exit implementation with a semihosting version, crucial for QEMU emulator support.
  • Improved llgo compilation flexibility: The change enables llgo to compile the semihosting _exit as a separate object file, which can then override the default implementation as needed.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the _exit function by moving it from syscalls.c to a new, separate file syscalls_exit.c. This is a sensible refactoring that improves modularity and will allow the function to be more easily overridden for different build configurations, such as for QEMU support. The change is clear and well-motivated. I have one minor suggestion to improve the code by explicitly marking an unused function parameter.

@luoliwoshang
Copy link
Member Author

@MeteorsLiu

* with semihosting version for QEMU emulator support.
*/

void
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMPORTANT: This function should be marked as __attribute__((weak)) to enable the stated goal of "easy replacement with semihosting version for QEMU emulator support."

Following the pattern in syscalls.c:28 where _write is declared weak, this should be:

Suggested change
void
__attribute__((weak))
void

Without the weak attribute, semihosting implementations cannot override this function, defeating the purpose of this refactoring.

@xgopilot
Copy link

xgopilot bot commented Jan 8, 2026

CRITICAL: The Makefile.inc was not updated to include the new syscalls_exit.c file. This will cause build failures.

Please add syscalls_exit.c to the build sources in libgloss/riscv/esp/Makefile.inc:

%C%_libsys_board_a_SOURCES = \
	%D%/esp_board.c \
	%D%/syscalls.c \
	%D%/syscalls_exit.c

Without this change, the _exit symbol will be undefined during linking.

@xgopilot
Copy link

xgopilot bot commented Jan 8, 2026

Code Review Summary

The refactoring concept is sound—separating _exit enables flexible replacement for QEMU semihosting. However, two critical issues must be addressed:

  1. Missing Makefile update: Build will fail without adding syscalls_exit.c to sources
  2. Missing weak attribute: Function cannot be overridden as intended without __attribute__((weak))

These prevent the PR from achieving its stated goal. Please see inline comments for details.

@MeteorsLiu MeteorsLiu merged commit 8559625 into goplus:main Jan 9, 2026
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.

2 participants