diff --git a/src/cmd/compile/internal/ssa/ssa2llvm.go b/src/cmd/compile/internal/ssa/ssa2llvm.go index 9238b5eb2738d4..76d41db9fe3feb 100644 --- a/src/cmd/compile/internal/ssa/ssa2llvm.go +++ b/src/cmd/compile/internal/ssa/ssa2llvm.go @@ -50,7 +50,7 @@ const goResultsTupleAttr = "go_results_tuple" const goGCStrategy = "goallc" const goGCLeafFunctionAttr = "gc-leaf-function" const goStackGrowthStatepointAttr = "go-stack-growth-statepoint" -const goNilCheckMetadata = "goallc.nilcheck" +const goNilCheckAnnotation = "goallc.nilcheck" const llvmFramePointerAttr = "frame-pointer" const llvmFramePointerNonLeaf = "non-leaf" @@ -1055,7 +1055,10 @@ func (lfc *LLVMFuncContext) GenLV(v *Value) llvm.Value { // encounter this load through a pointer-containing static alloca, but // must continue to reject every unmarked volatile or atomic access to // such storage. - check.SetMetadata(GlobalCtxt.MDKindID(goNilCheckMetadata), GlobalCtxt.MDNode(nil)) + check.SetMetadata( + GlobalCtxt.MDKindID("annotation"), + GlobalCtxt.MDNode([]llvm.Metadata{GlobalCtxt.MDString(goNilCheckAnnotation)}), + ) lVal = p case OpStore: lVal = lfc.b.CreateStore(arg1(), arg0()) diff --git a/src/cmd/llvmplugin/CMakeLists.txt b/src/cmd/llvmplugin/CMakeLists.txt index 5e0c6254bb612e..02d51c4fed8f06 100644 --- a/src/cmd/llvmplugin/CMakeLists.txt +++ b/src/cmd/llvmplugin/CMakeLists.txt @@ -370,6 +370,18 @@ if(BUILD_TESTING) "${CMAKE_CURRENT_SOURCE_DIR}/testdata/alloca-pointer-roots.ll" ) + add_test( + NAME GoALLCStatepoints.AllocaNilCheckO2 + COMMAND + "${Python3_EXECUTABLE}" + "${CMAKE_CURRENT_SOURCE_DIR}/testdata/check-alloca-nilcheck-o2.py" + --llc "${GOALLC_LLC_EXECUTABLE}" + --opt "${GOALLC_OPT_EXECUTABLE}" + --plugin "$" + --input + "${CMAKE_CURRENT_SOURCE_DIR}/testdata/alloca-nilcheck-o2.ll" + ) + add_test( NAME GoALLCStatepoints.MalformedAllocaPointerMapsFail COMMAND diff --git a/src/cmd/llvmplugin/GoALLCStatepoints.cpp b/src/cmd/llvmplugin/GoALLCStatepoints.cpp index 35be200bd4b564..991045f2f03c8e 100644 --- a/src/cmd/llvmplugin/GoALLCStatepoints.cpp +++ b/src/cmd/llvmplugin/GoALLCStatepoints.cpp @@ -38,7 +38,7 @@ namespace { constexpr StringLiteral GoALLCGCName = "goallc"; constexpr StringLiteral GCLeafAttr = "gc-leaf-function"; constexpr StringLiteral GoResultsTupleAttr = "go_results_tuple"; -constexpr StringLiteral GoNilCheckMD = "goallc.nilcheck"; +constexpr StringLiteral GoNilCheckAnnotation = "goallc.nilcheck"; // This strategy exists for statepoint verification and lowering. GoALLC owns // statepoint insertion, so UseRS4GC deliberately remains false. @@ -517,6 +517,24 @@ std::string allocaLeafName(AllocaInst &Alloca, const PointerAllocaLeaf &Leaf) { return Name; } +bool hasAnnotation(const Instruction &I, StringRef Name) { + MDNode *Annotations = I.getMetadata(LLVMContext::MD_annotation); + if (!Annotations) + return false; + for (const MDOperand &Operand : Annotations->operands()) { + if (auto *String = dyn_cast_or_null(Operand.get()); + String && String->getString() == Name) + return true; + auto *Tuple = dyn_cast_or_null(Operand.get()); + if (Tuple && any_of(Tuple->operands(), [Name](const MDOperand &Nested) { + auto *String = dyn_cast_or_null(Nested.get()); + return String && String->getString() == Name; + })) + return true; + } + return false; +} + Error validatePointerAllocaAccesses(AllocaInst &Alloca, Function &F) { for (Instruction &I : instructions(F)) { if (auto *Intrinsic = dyn_cast(&I); @@ -534,11 +552,9 @@ Error validatePointerAllocaAccesses(AllocaInst &Alloca, Function &F) { bool UnsupportedAccess = false; if (auto *Load = dyn_cast(&I)) { Address = Load->getPointerOperand(); - MDNode *NilCheck = Load->getMetadata(GoNilCheckMD); bool IsFrontendNilCheck = - NilCheck && NilCheck->getNumOperands() == 0 && Load->isVolatile() && - !Load->isAtomic() && Load->getType()->isIntegerTy(8) && - Load->getAlign() == Align(1); + hasAnnotation(*Load, GoNilCheckAnnotation) && Load->isVolatile() && + !Load->isAtomic() && Load->getType()->isIntegerTy(8); UnsupportedAccess = Load->isAtomic() || (Load->isVolatile() && !IsFrontendNilCheck); } else if (auto *Store = dyn_cast(&I)) { diff --git a/src/cmd/llvmplugin/README.md b/src/cmd/llvmplugin/README.md index 7d285328847a21..1cada4f23b6d43 100644 --- a/src/cmd/llvmplugin/README.md +++ b/src/cmd/llvmplugin/README.md @@ -136,9 +136,10 @@ and relocation. Constants are not roots in the general SSA model. Alloca records describe the memory layout even when a pointer slot currently contains null. Address passing to a callee is supported. A volatile byte load carrying the -compiler-owned empty `!goallc.nilcheck` marker is recognized as an SSA -`OpNilCheck` and remains in place for its faulting semantics; this does not -represent a volatile read of the pointer storage. Dynamic, multiple-element, +standard `!annotation !{!"goallc.nilcheck"}` marker is recognized as an SSA +`OpNilCheck` and remains in place for its faulting semantics; SROA preserves +this annotation when it rebuilds the load, and the load does not represent a +volatile read of the pointer storage. Dynamic, multiple-element, scalable, or realigned allocas, pointer vectors, lifetime markers, every unmarked volatile access, and every atomic access fail closed until their frame-home and update semantics are explicit. diff --git a/src/cmd/llvmplugin/testdata/alloca-nilcheck-o2.ll b/src/cmd/llvmplugin/testdata/alloca-nilcheck-o2.ll new file mode 100644 index 00000000000000..3f757c092e013a --- /dev/null +++ b/src/cmd/llvmplugin/testdata/alloca-nilcheck-o2.ll @@ -0,0 +1,16 @@ +target triple = "aarch64-unknown-linux-goobj" + +declare goabiinternal void @safepoint() + +define goabiinternal ptr @nilcheck_sroa(ptr returned %value) "go-stack-growth-statepoint" gc "goallc" { +entry: + %slot = alloca ptr, align 8 + store ptr null, ptr %slot, align 8 + %nilcheck = load volatile i8, ptr %slot, align 1, !annotation !0 + call goabiinternal void @safepoint() + store ptr %value, ptr %slot, align 8 + %result = load ptr, ptr %slot, align 8 + ret ptr %result +} + +!0 = !{!"goallc.nilcheck"} diff --git a/src/cmd/llvmplugin/testdata/alloca-pointer-roots.ll b/src/cmd/llvmplugin/testdata/alloca-pointer-roots.ll index 0c7d3dbe86103a..9d67bf8268088d 100644 --- a/src/cmd/llvmplugin/testdata/alloca-pointer-roots.ll +++ b/src/cmd/llvmplugin/testdata/alloca-pointer-roots.ll @@ -16,7 +16,7 @@ define goabiinternal ptr @pointer_slot(ptr %pointer) "go-stack-growth-statepoint entry: %slot = alloca ptr, align 8 store ptr %pointer, ptr %slot, align 8 - %nilcheck = load volatile i8, ptr %slot, align 1, !goallc.nilcheck !0 + %nilcheck = load volatile i8, ptr %slot, align 1, !annotation !0 call goabiinternal void @safepoint() [ "deopt"(i64 7) ] %result = load ptr, ptr %slot, align 8 ret ptr %result @@ -196,4 +196,4 @@ entry: ptr @alloca_readonly_and_readnone ], section "llvm.metadata" -!0 = !{} +!0 = !{!"goallc.nilcheck"} diff --git a/src/cmd/llvmplugin/testdata/check-alloca-nilcheck-o2.py b/src/cmd/llvmplugin/testdata/check-alloca-nilcheck-o2.py new file mode 100644 index 00000000000000..896de96a5b1324 --- /dev/null +++ b/src/cmd/llvmplugin/testdata/check-alloca-nilcheck-o2.py @@ -0,0 +1,54 @@ +#!/usr/bin/env python3 + +import argparse +import re +import subprocess +import sys + + +def fail(message): + print(f"alloca nilcheck O2 check failed: {message}", file=sys.stderr) + raise SystemExit(1) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--llc", required=True) + parser.add_argument("--opt", required=True) + parser.add_argument("--plugin", required=True) + parser.add_argument("--input", required=True) + args = parser.parse_args() + + optimized = subprocess.run( + [args.opt, "-passes=default", "-S", "-o", "-", args.input], + capture_output=True, + text=True, + ) + if optimized.returncode != 0: + fail(f"opt failed:\n{optimized.stdout}{optimized.stderr}") + if not re.search( + r"load volatile i8, ptr %slot, align 8, !annotation !\d+", + optimized.stdout, + ): + fail("SROA replacement load lost the frontend nilcheck annotation") + + lowered = subprocess.run( + [ + args.llc, + f"-load-pass-plugin={args.plugin}", + "-verify-machineinstrs", + "-filetype=null", + "-o", + "-", + "-", + ], + input=optimized.stdout, + capture_output=True, + text=True, + ) + if lowered.returncode != 0: + fail(f"llc failed:\n{lowered.stdout}{lowered.stderr}") + + +if __name__ == "__main__": + main() diff --git a/test/codegen/issue59297.go b/test/codegen/issue59297.go index 90ed6ebfa4eb40..e2987bb6285704 100644 --- a/test/codegen/issue59297.go +++ b/test/codegen/issue59297.go @@ -8,7 +8,7 @@ package codegen // LLVM-LABEL: define goabiinternal void @codegen.f(i64 %x, i64 %y, ptr %p) // LLVM: call goabiinternal void @codegen.h(i64 8, i64 %x) -// LLVM: load volatile i8, ptr %p, align 1, !goallc.nilcheck !{{[0-9]+}} +// LLVM: load volatile i8, ptr %p, align 1, !annotation !{{[0-9]+}} // LLVM: store i64 %y, ptr %p, align 4 func f(x, y int, p *int) {