llvm: bind LLVM 22 pointer address extraction - #53
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: LLVM 22 ptrtoaddr bindings
Solid, well-scoped addition. It follows the repo's version-gated split-file convention (mirroring switch_llvm22.go), the C++ shim is guarded with #if LLVM_VERSION_MAJOR >= 22, doc comments are accurate against LLVM's ptrtoaddr semantics, and the tests are strong — TestPtrToAddrDoesNotExposeProvenance empirically proves the provenance distinction from ptrtoint by running default<O2> and asserting the load is eliminated only for ptrtoaddr.
Findings are inline. A few cross-file notes below.
Build-tag / pre-22 contract (worth a maintainer decision). Unlike switch, there is no ptrtoaddr_pre22.go stub. ptrtoaddr_llvm22.go builds under the default tag set (!llvm14 && ... && !llvm21), so if someone builds with no version tag against LLVM ≤ 21 headers, this file still compiles and references C.LLVMPtrToAddr (missing from the older enum → compile error) plus the C++ helpers (guarded out → link error). This matches the pre-existing "default means latest/22" assumption in llvm_config_llvm22.go, so it may be acceptable — but the Go file keys off build tags while the C++ keys off LLVM_VERSION_MAJOR, and the two disagree in that one case. Consider either documenting the default-tag contract or adding a ptrtoaddr_pre22.go stub for parity with switch.
unwrap<Constant> in ConstPtrToAddr (informational). LLVMGoConstPtrToAddr uses unwrap<Constant>(V) (i.e. cast<Constant>), which in LLVM's default NDEBUG build is an unchecked downcast — passing a non-Constant value is UB. This is identical to every other Const* binding (e.g. LLVMConstPtrToInt, IRBindings.cpp) and the upstream LLVM-C trust model, so no change is required; noted only for completeness.
Performance and documentation passes found no issues.
| @@ -0,0 +1,18 @@ | |||
| #ifndef LLVM_BINDINGS_GO_PTRTOADDR_H | |||
There was a problem hiding this comment.
[P2] Missing LLVM license header in new C++ files
Every existing binding file (IRBindings.h, SupportBindings.h, TargetBindings.h, etc.) opens with the standard //===- ... ===// Apache-2.0-WITH-LLVM-exception header block. Both PtrToAddrBindings.h and PtrToAddrBindings.cpp omit it. For an LLVM sub-project this header is effectively mandatory — please add the same block to both new files.
| @@ -0,0 +1,18 @@ | |||
| #include "PtrToAddrBindings.h" | |||
There was a problem hiding this comment.
[P2] Missing LLVM license header
Same as the header file: this .cpp is missing the standard //===- ... ===// Apache-2.0-WITH-LLVM-exception block that every other binding source in the repo carries.
| import "C" | ||
| import "unsafe" | ||
|
|
||
| const PtrToAddr Opcode = C.LLVMPtrToAddr |
There was a problem hiding this comment.
[P3] PtrToAddr Opcode const lives apart from the others in ir.go
All other Opcode constants — including the closely related PtrToInt Opcode = C.LLVMPtrToInt — live in the single const block in ir.go, so placing PtrToAddr here breaks discoverability. Keeping it separate is a genuine necessity (the enum only exists in the LLVM 22 C API, and ir.go is compiled unconditionally). Consider a one-line comment noting it is kept here rather than in ir.go because C.LLVMPtrToAddr only exists in LLVM 22+, to save the next maintainer the investigation.
| fn := AddFunction(mod, tc.name, FunctionType(tc.addr, []Type{tc.ptr}, false)) | ||
| builder.SetInsertPointAtEnd(ctx.AddBasicBlock(fn, "entry")) | ||
| addr := builder.CreatePtrToAddr(fn.Param(0), "addr") | ||
| if addr.Type() != tc.addr || addr.InstructionOpcode() != PtrToAddr { |
There was a problem hiding this comment.
[P3] Combined assertion shares one error message
This check tests two distinct properties (result type and opcode) but reports a single "unexpected ptrtoaddr" message. On failure the developer can't immediately tell whether the type or the opcode was wrong. Splitting into two checks, or including the expected-vs-actual type/opcode in the message, would improve diagnosability. Minor — the printed addr.String() already gives a strong hint.
2 participants
LLVM 22's
ptrtoaddrobserves a pointer's address without exposing its provenance. AddBuilder.CreatePtrToAddrusing the module DataLayout to select the address-space index width, plus the opcode and constant-expression constructor so consumers can preserve the operation when cloning IR.The new APIs are available only for LLVM 22. The small C++ bridge is version guarded because the LLVM C API exposes the opcode but lacks dedicated builder and constant constructors.
Validation on macOS arm64:
go test ./...with LLVM 22.1.8.go test -tags=llvm21 ./...andgo test -tags=llvm19 ./....ptrtoint.Other platforms and LLVM versions remain subject to CI.