ssa: impl builtin alignof offsetof

[visualfc]

implement builtin unsafe.Alignof unsafe.Offsetof instance of generic function

[niupilot]

Code Review Complete ✅

Summary

This PR implements support for unsafe.Alignof and unsafe.Offsetof builtin functions in the LLGO compiler. The implementation adds these functions to the builtin call handler and includes comprehensive tests with generic types.

Detailed Analysis

Architecture & Design ✅

• Clean integration: The new functions integrate well with the existing builtin function infrastructure in ssa/expr.go
• Consistent patterns: Follows the same pattern as unsafe.Sizeof with proper comment structure and implementation style
• Generic support: Properly handles generic types as evidenced by the test cases using M[bool] and M[int64]

Implementation Quality ✅

Strengths:

• LLVM integration: Correctly uses b.Prog.td.ABITypeAlignment() and b.Prog.td.ElementOffset() for platform-specific calculations
• Error handling: Offsetof includes proper validation with a descriptive panic message for invalid arguments
• Test coverage: Comprehensive test cases verify alignment and offset calculations for different types

Code Quality:

• Follows existing code style and patterns
• Appropriate use of LLVM introspection methods
• Clear, descriptive comments

Potential Issues ⚠️

1. Offsetof validation robustness (ssa/expr.go:1266-1276):

   • The current validation chain relies on specific LLVM instruction patterns
   • Could potentially fail with future LLVM optimizations or different code generation patterns
   • Consider adding more robust validation or clearer error messages

2. Test expectations verification (cl/_testrt/tpunsafe/in.go:19-30):

   • The hardcoded expected values (1, 8, 1) and (8, 16, 8) should be verified against actual platform ABI specifications
   • Consider documenting the platform assumptions or making tests more portable

Security & Safety ✅

• No security concerns identified
• Proper use of unsafe operations within expected bounds
• Error handling prevents undefined behavior

Documentation & Consistency ✅

• Comments match existing style ("instance of generic function")
• Function names and behavior align with Go's unsafe package
• Test code is clear and well-structured

Recommendations

1. Consider enhancing Offsetof validation to handle edge cases more gracefully
2. Document platform assumptions in test expectations
3. Consider adding negative test cases for invalid Offsetof arguments

Overall Assessment

This is a well-implemented feature that correctly adds support for two important unsafe package functions. The code follows project conventions, includes proper testing, and integrates cleanly with the existing codebase. The implementation correctly handles the low-level LLVM details required for these operations.

Recommendation: Approve - This PR successfully implements the required functionality with good code quality and appropriate testing.

---

👉 Want me to re-run this review? Just type /review in the comments! For more usage examples, visit CodeAgent GitHub Repository.

[gemini-code-assist]

Summary of Changes

Hello @visualfc, I'm Gemini Code Assist¹! 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 integrates the unsafe.Alignof and unsafe.Offsetof built-in functions into the SSA compiler, allowing for precise determination of memory alignment and field offsets within Go programs. This enhancement is critical for low-level memory manipulation and ensures accurate type layout information, particularly for generic types, by leveraging LLVM's data layout capabilities.

Highlights

• Built-in Alignof Implementation: The BuiltinCall function in ssa/expr.go now includes support for unsafe.Alignof. This function determines the ABI type alignment of a given type by querying the LLVM DataLayout, ensuring correct memory alignment calculations.
• Built-in Offsetof Implementation: Support for unsafe.Offsetof has been added to the BuiltinCall function. This implementation specifically handles selector expressions (e.g., struct.field) by analyzing the generated LLVM LoadInst and GetElementPtrInst to calculate the byte offset of a field within a struct. It includes validation to ensure the argument is a valid selector expression.
• New Test Case for unsafe Operations with Generics: A new test file cl/_testrt/tpunsafe/in.go has been added. This test verifies the correctness of unsafe.Alignof and unsafe.Offsetof when applied to generic struct types, ensuring that these built-in functions work as expected across different type instantiations.
• LLVM IR Output for unsafe Test: The corresponding LLVM IR output for the new tpunsafe test, cl/_testrt/tpunsafe/out.ll, has been included. This file demonstrates the generated low-level code for the unsafe.Alignof and unsafe.Offsetof operations, confirming their proper translation to LLVM.

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. ↩

[niupilot]

Code Review Summary

This PR successfully implements unsafe.Alignof and unsafe.Offsetof builtin functions with good integration into the existing codebase. The implementation follows established patterns and includes comprehensive testing.

Key Strengths:

• Clean integration with existing builtin function infrastructure
• Proper LLVM API usage for platform-specific calculations
• Comprehensive test coverage with generic types
• Consistent code style and documentation

Areas for Improvement:

• Enhanced robustness in Offsetof validation
• Better documentation of platform assumptions in tests

Overall, this is a well-implemented feature that adds important functionality to the LLGO compiler.

[gemini-code-assist]

Code Review

This pull request implements the unsafe.Alignof and unsafe.Offsetof built-ins. The implementation for Alignof seems correct. The Offsetof implementation is a bit complex and relies on specific LLVM IR patterns, but it works for the provided test case. I've suggested a small refactoring to improve its readability. Additionally, the new test file cl/_testrt/tpunsafe/in.go has a minor issue in its error reporting where println is used with format-like strings, which I've also commented on.

[codecov]

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.10%. Comparing base (18e0365) to head (7323187).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
ssa/expr.go	92.85%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1318   +/-   ##
=======================================
  Coverage   90.10%   90.10%           
=======================================
  Files          43       43           
  Lines       12562    12576   +14     
=======================================
+ Hits        11319    11332   +13     
- Misses       1087     1088    +1     
  Partials      156      156           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.