Skip to content

llvm: make LLVM 22 the default - #51

Open
zhouguangyuan0718 wants to merge 2 commits into
xgo-dev:xgofrom
zhouguangyuan0718:codex/default-llvm22-20260901
Open

llvm: make LLVM 22 the default#51
zhouguangyuan0718 wants to merge 2 commits into
xgo-dev:xgofrom
zhouguangyuan0718:codex/default-llvm22-20260901

Conversation

@zhouguangyuan0718

Copy link
Copy Markdown

Summary

  • select LLVM 22 when no explicit LLVM version tag is present
  • keep LLVM 21 available through the explicit llvm21 tag
  • align the LLVM 22-only switch operand API implementation with the new default
  • move default CI coverage to LLVM 22, including Fedora 44
  • update the README default and Windows pkg-config example

Validation

  • go test -count=1 -v ./... with no version tag against LLVM 22.1.8: pass
  • go test -count=1 -tags=llvm21 ./... against LLVM 21.1.8: pass
  • the no-tag run reports linked LLVM 22.1.8 and exercises TestSwitchCaseValue, TargetMachine options, PassBuilder and IR tests

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review: default LLVM 21 → 22

This PR cleanly bumps the default LLVM version from 21 to 22. The //go:build partition across the config/switch files is mutually exclusive and complete for modern Go: any single llvmNN tag selects exactly one llvm_config_* file and one switch_* file, and the tagless (default) case correctly resolves to LLVM 22 via the negative-clause fallback. The README, CI matrix, and pkg-config references were all updated consistently, and the new Windows LLVM 22 entry preserves the pinned-SHA256 download-verify pattern.

Security and performance passes found nothing to block (no runtime logic changes). One low-severity build-tag consistency note is inline below.

Notes (non-blocking, out of scope)

  • .github/workflows/go.yml (Linux job): wget ... | sudo apt-key add - trusts the LLVM key without fingerprint pinning and uses the deprecated apt-key. Pre-existing (line unchanged by this PR); flagging only for awareness.
  • README.markdown "Currently supported" list omits FreeBSD despite FreeBSD cgo flags in the config files. Pre-existing, unrelated to this change.

Comment thread llvm_config_llvm22.go
@@ -1,4 +1,4 @@
//go:build !byollvm && llvm22
//go:build !byollvm && (llvm22 || (!llvm14 && !llvm15 && !llvm16 && !llvm17 && !llvm18 && !llvm19 && !llvm20 && !llvm21))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P3] Build-tag consistency: //go:build-only files vs go.mod 1.14

This file's build constraint now uses only //go:build, with no legacy // +build fallback — same for switch_llvm22.go and switch_pre22.go. go.mod declares go 1.14, and Go toolchains before 1.17 ignore //go:build lines entirely. On such a toolchain these constraints would be dropped, causing duplicate symbols (e.g. two GetSwitchCaseValue definitions) and a build failure.

This is a pre-existing condition (the switch/llvm22 files already lacked // +build in the base), not a regression introduced here — and CI pins Go 1.22, so it's invisible to the test matrix. But since this PR is already editing these constraint lines, it's a good moment to make the intent explicit: either add matching // +build lines to these three files (the sibling llvm_config_llvm19/20/21.go still carry both forms), or bump go.mod to go 1.17 and drop the now-dead // +build lines everywhere for a uniform style. Low severity / optional.

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.

1 participant