stdlib-opt: trim runtime/internal/lib patches#1621
Conversation
Summary of ChangesHello @cpunion, 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 represents a major architectural shift aimed at improving LLGo's compatibility and maintainability by enabling more direct reuse of the upstream Go standard library. By introducing compiler intrinsics and carefully crafted runtime hooks, LLGo can now leverage existing Go standard library code for critical functionalities like syscalls and math operations, significantly reducing the need for custom, LLGo-specific implementations. This change simplifies the codebase, aligns LLGo more closely with upstream Go, and enhances the robustness of its runtime environment. Highlights
🧠 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. Changelog
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring that trims down the runtime/internal/lib patches, enabling more of the standard library to be used directly. The introduction of syscall intrinsics and the corresponding compiler support in cl/instr.go is a solid approach to handle platform-specific syscalls. The addition of Dockerfiles for different Go versions and helper scripts will improve the development and testing workflow. I have a few minor suggestions for improvement.
Code Review SummaryThis PR introduces valuable syscall intrinsics functionality ( Key ObservationsPositives:
Areas for improvement:
See inline comments for specific suggestions. |
e6a2517 to
0c38765
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1621 +/- ##
==========================================
- Coverage 91.10% 91.08% -0.03%
==========================================
Files 45 45
Lines 11921 12041 +120
==========================================
+ Hits 10861 10967 +106
- Misses 886 897 +11
- Partials 174 177 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add doc comments for zeroResult/syscallFnSig/syscallFnSigFixed/syscallErrno/syscallIntrinsic to make the low-level lowering easier to maintain.
73d9fd0 to
8a7412e
Compare
Move dev/ docker/CI tooling updates to chore/dev-dockerfiles (separate PR).
Document the current runtime/internal/lib alt-package set and removal candidates, with notes on upstream assembly dependency and plan9-asm integration targets.
b664177 to
edcb511
Compare
…nal-lib # Conflicts: # runtime/build.go # runtime/internal/lib/crypto/internal/boring/sig/sig.go
edcb511 to
12046e4
Compare
Links
Summary
runtime/internal/libdown to a small, explicit set of alt packages needed by llgo ABI/runtime and syscall path.runtime/_overlay/math) so stdlib can build without relying on upstream asm.Key changes
syscall.syscall*to llgo intrinsics and normalize errno handling.internal/runtime/syscallalt withSyscall6backed by libcsyscall.archExp/archExp2via libc and disable asm-only paths where needed.Alt packages (source of truth:
runtime/build.gohasAltPkg)crypto/internal/boring/sighash/crc32internal/abiinternal/bytealginternal/cpuinternal/reflectliteinternal/runtime/atomicinternal/runtime/mapsinternal/runtime/syscallinternal/weakreflectruntimeruntime/internal/syscallsyncsync/atomicsyscall/jsuniqueweakNotes
runtime/internal/libis not treated as an alt package on this branch (e.g.internal/itoa,internal/syscall, mostsyscall/*, mostcrypto/*, etc.).Testing
_demo/go/{goimporter-1389,gotypes,maphash,netip,randcrypt,oswritestring,reflectmake,sysexec,texttemplate}_demo/py/tensor,_demo/c/cargs