Skip to content

lto: analyze aggregate MethodByName string arguments#2159

Open
cpunion wants to merge 1 commit into
xgo-dev:mainfrom
cpunion:codex/lto-aggregate-string-analysis
Open

lto: analyze aggregate MethodByName string arguments#2159
cpunion wants to merge 1 commit into
xgo-dev:mainfrom
cpunion:codex/lto-aggregate-string-analysis

Conversation

@cpunion

@cpunion cpunion commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Unblocks #2143.

Problem

The MethodByName LTO pass assumed that every marked Go string argument had been lowered into separate (ptr, len) parameters. LLGo's arm64 LP64 C ABI path can instead keep the 16-byte Go string as a {ptr, len} aggregate. In that form the pass could not recover direct, concatenated, sliced, or forwarded constant names, so it left the generic reflect marker in place and prevented the intended named-method pruning.

Changes

  • consume one logical string argument in either aggregate or split ABI form
  • reuse that decoding for direct MethodByName calls, runtime.StringCat, runtime.StringSlice2, and forwarded function parameters
  • preserve the generic marker whenever a name cannot be resolved
  • add Go fixtures covering direct, concat, slice, forwarding, linked execution, actual unused-method pruning, and unknown-name fallback
  • derive the aggregate-ABI assertions from generated Go modules rather than hand-written LLVM IR

The focused aggregate test generates frontend IR with automatic C ABI lowering disabled, applies LLGo's arm64 C ABI transformer, and runs the LTO pass. One module checks the four known-name markers without a generic value marker. A separate module checks conservative unknown-name fallback because a generic marker intentionally retains all matching methods and would invalidate an unused-method symbol assertion. The normal plugin runtime and symbol suites exercise the linked split-ABI path and verify that Known.Drop is absent from the final binary.

This PR changes analysis only; it does not change either ABI.

Validation

  • negative check: the Go-generated aggregate test fails with the main plugin at the missing Concat marker and passes with this PR
  • macOS arm64, LLVM 19: clean plugin build and both generated aggregate modules pass
  • Ubuntu arm64 container, capped at 2 CPUs / 15 GiB: both generated aggregate modules pass
  • Ubuntu arm64: linked runtime fixture passes and the final symbol table excludes Known.Drop
  • Ubuntu arm64: complete existing LTO-plugin runtime and symbol suites; the new fixture passes in the runtime suite
  • Ubuntu arm64: CI-equivalent plugin coverage command, 52.7% aggregate coverage for the instrumented Go packages
  • prior CI: Dev LTO GlobalDCE, both coverage platforms, and Codecov patch coverage pass

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cpunion
cpunion force-pushed the codex/lto-aggregate-string-analysis branch from 5abb561 to bbc6c11 Compare July 23, 2026 02:12
@cpunion cpunion changed the title lto: analyze MethodByName strings across aggregate and split ABI lto: analyze aggregate MethodByName string arguments Jul 23, 2026
Comment thread ltoplugin/LLGOReflectMethodByNamePass.cpp Outdated
@cpunion
cpunion force-pushed the codex/lto-aggregate-string-analysis branch from bbc6c11 to f2fc976 Compare July 24, 2026 05:13
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