build: emit LLVM objects directly from modules by zhouguangyuan0718 · Pull Request #1761 · goplus/llgo

zhouguangyuan0718 · 2026-03-29T07:58:56Z

Summary

replace the .ll -> clang -c path for LLGo-generated modules with in-process LLVM object emission
route translated plan9 asm modules and link-time .ll inputs through the same LLVM emission helper
map existing clang target/codegen flags onto LLVM target machine and pass pipeline settings
erase replaced cabi call instructions after rewriting uses to avoid leaving dead instructions behind

Testing

go test ./internal/build -run TestRun
go test ./ssa -run TestFromTestlibgo/atomic
go test ./internal/clang
go test ./internal/cabi

gemini-code-assist

Code Review

This pull request refactors the build system to utilize LLVM Go bindings for emitting object files and IR, replacing several external clang invocations. It introduces a new llvm_emit.go file for LLVM configuration and emission logic, and expands the Target structure to support detailed LLVM parameters like Triple, CPU, and relocation models. Review feedback highlights a bug in ssa/target.go where -O0 optimization is incorrectly overridden by a default value check, and suggests using defer for LLVM module disposal in internal/build/plan9asm.go to simplify resource management.

gemini-code-assist · 2026-03-29T08:02:15Z

ssa/target.go

+	if optLevel == 0 {
+		optLevel = llvm.CodeGenLevelDefault
+	}


This logic introduces a bug. llvm.CodeGenLevelNone is 0, so if a user specifies -O0 (which sets CodeGenLevel to 0), this block will incorrectly override it to llvm.CodeGenLevelDefault (2). This means -O0 will not correctly disable optimizations at the codegen level. The zero value for CodeGenOptLevel is a valid setting, so this check for 0 to apply a default is problematic.

The default optimization level should be set when the Target configuration is initialized if no optimization flags are provided, and this block should be removed.

gemini-code-assist · 2026-03-29T08:02:15Z

internal/build/plan9asm.go

+			mod.Dispose()
 			os.Remove(objPath)
-			return nil, fmt.Errorf("%s: clang compile asm ll for %s: %w", pkg.PkgPath, sfile, err)
+			return nil, fmt.Errorf("%s: emit asm object for %s: %w", pkg.PkgPath, sfile, err)
 		}
+		mod.Dispose()


The explicit calls to mod.Dispose() on both the error path (line 102) and the success path (line 106) can be simplified. By using a defer mod.Dispose() statement after mod is initialized (around line 71), you can ensure that Dispose is always called, which makes the code cleaner and less error-prone. This would also allow removing the mod.Dispose() call in another error path at line 86.

xgopilot · 2026-03-29T08:06:13Z

ssa/target.go

+	optLevel := p.CodeGenLevel
+	if optLevel == 0 {
+		optLevel = llvm.CodeGenLevelDefault
+	}


Bug: CodeGenLevelNone (i.e. -O0) has the integer value 0, which is the same as the zero value of CodeGenOptLevel. This guard makes it impossible to honor an explicit -O0 request — it silently promotes to CodeGenLevelDefault (O2). The IR pass pipeline will correctly run default<O0>, but the backend codegen will use O2 optimization.

Consider using a pointer (*llvm.CodeGenOptLevel) or a separate boolean to distinguish "unset" from "O0".

xgopilot · 2026-03-29T08:06:15Z

ssa/target.go

+		spec.CPU = p.CPU
+		spec.Features = p.Features
+		return
+	}


When Triple is set but CPU and Features are empty, this returns an empty TargetSpec.CPU and TargetSpec.Features, bypassing all the architecture-specific defaults below (e.g. "x86-64" with SSE2 features for amd64, "generic" with "+neon" for arm64). The old clang -c path would infer sensible defaults from the triple alone. With in-process emission, empty CPU/Features may produce less optimal or functionally different code.

Consider falling through to the CPU/Features override block at lines 160-165 even when Triple is set, or at minimum defaulting CPU to "generic".

xgopilot · 2026-03-29T08:06:17Z

internal/build/plan9asm.go

@@ -117,15 +95,15 @@ func compilePkgSFiles(ctx *context, aPkg *aPackage, pkg *packages.Package, verbo
 		objPath := objFile.Name()


If os.CreateTemp fails here, the function returns without calling mod.Dispose(). The error paths at lines 86 and 102 correctly dispose the module, but this one leaks it.

Consider using defer mod.Dispose() right after obtaining the module (after line 79) to uniformly cover all exit paths, rather than manual Dispose calls on each error branch.

xgopilot · 2026-03-29T08:06:19Z

internal/build/llvm_emit.go

+func (c *context) emitModuleObject(pkgPath string, mod gllvm.Module, objPath string) error {
+	cfg := c.llvmCompileConfig()
+
+	mod.SetTarget(cfg.target.Spec().Triple)


emitModuleObject mutates the caller's module in place (SetTarget, SetDataLayout, and potentially runs optimization passes). In exportObject and plan9asm, the current call ordering (write IR first, then emit) happens to be correct, but this is a fragile implicit contract. A future caller reading from the module after emission would get mutated IR. Worth a doc comment on the method warning that it modifies the module.

xgopilot · 2026-03-29T08:06:21Z

internal/build/llvm_emit.go

+				i++
+				cfg.target.CPU = flags[i]
+			}
+		case strings.HasPrefix(arg, "-mcpu="):


Nit: -march specifies a target architecture (e.g. armv7-a, haswell) while -mcpu specifies a CPU model — these are distinct concepts in LLVM. Mapping both to cfg.target.CPU conflates them. LLVM's CreateTargetMachine CPU parameter expects a CPU name, not an architecture string. Passing an architecture string may silently produce suboptimal code on some targets.

xgopilot · 2026-03-29T08:06:24Z

internal/build/llvm_emit.go

+}
+
+func (c *context) emitModuleObject(pkgPath string, mod gllvm.Module, objPath string) error {
+	cfg := c.llvmCompileConfig()


llvmCompileConfig() is recomputed on every call to emitModuleObject — this re-reads env vars, re-instantiates a clang.Cmd, and re-parses flags, even though the config is invariant within a build. For large builds with many packages, consider computing this once and caching it on the context.

xgopilot · 2026-03-29T08:06:26Z

internal/clang/clang.go

+func (c *Cmd) LinkerFlags() []string {
+	return c.mergeLinkerFlags()
+}
+


LinkerFlags() has no callers in the codebase. Consider removing it until it's needed to avoid dead code.

xgopilot · 2026-03-29T08:06:38Z

Good refactoring — eliminating the .ll serialization roundtrip and clang subprocess is a solid architectural improvement. The new llvm_emit.go is well-organized and the flag parsing is thorough.

Key issues to address:

Bug: -O0 cannot be distinguished from "unset" due to zero-value collision in CodeGenLevel, silently promoting to O2 codegen.
Resource leak: Missing mod.Dispose() on one error path in plan9asm.go. Consider defer instead of manual dispose.
Behavioral gap: Spec() early return when Triple is set skips CPU/Features defaults that clang would infer.
-march and -mcpu are conflated in flag parsing.

See inline comments for details.

- replace the .ll -> clang -c path for LLGo-generated modules with in-process LLVM object emission - route translated plan9 asm modules and link-time .ll inputs through the same LLVM emission helper - map existing clang target/codegen flags onto LLVM target machine and pass pipeline settings - erase replaced cabi call instructions after rewriting uses to avoid leaving dead instructions behind Testing: - go test ./internal/build -run TestRun - go test ./ssa -run TestFromTestlibgo/atomic - go test ./internal/clang

gemini-code-assist bot reviewed Mar 29, 2026

View reviewed changes

xgopilot bot reviewed Mar 29, 2026

View reviewed changes

zhouguangyuan0718 force-pushed the main-objfile branch from dc1e500 to 3cacb55 Compare March 29, 2026 08:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: emit LLVM objects directly from modules#1761

build: emit LLVM objects directly from modules#1761
zhouguangyuan0718 wants to merge 1 commit intogoplus:mainfrom
zhouguangyuan0718:main-objfile

zhouguangyuan0718 commented Mar 29, 2026 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Mar 29, 2026

Uh oh!

gemini-code-assist bot Mar 29, 2026

Uh oh!

xgopilot bot Mar 29, 2026

Uh oh!

xgopilot bot Mar 29, 2026

Uh oh!

xgopilot bot Mar 29, 2026

Uh oh!

xgopilot bot Mar 29, 2026

Uh oh!

xgopilot bot Mar 29, 2026

Uh oh!

xgopilot bot Mar 29, 2026

Uh oh!

xgopilot bot Mar 29, 2026

Uh oh!

xgopilot bot commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -117,15 +95,15 @@ func compilePkgSFiles(ctx context, aPkg aPackage, pkg *packages.Package, verbo
		objPath := objFile.Name()

Conversation

zhouguangyuan0718 commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zhouguangyuan0718 commented Mar 29, 2026 •

edited

Loading