cmd/llvmplugin: use standard annotations for nil checks

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())

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 "$<TARGET_FILE:GoALLCStatepoints>"
+      --input
+        "${CMAKE_CURRENT_SOURCE_DIR}/testdata/alloca-nilcheck-o2.ll"
+  )
+
   add_test(
     NAME GoALLCStatepoints.MalformedAllocaPointerMapsFail
     COMMAND

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<MDString>(Operand.get());
+        String && String->getString() == Name)
+      return true;
+    auto *Tuple = dyn_cast_or_null<MDTuple>(Operand.get());
+    if (Tuple && any_of(Tuple->operands(), [Name](const MDOperand &Nested) {
+          auto *String = dyn_cast_or_null<MDString>(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<IntrinsicInst>(&I);
@@ -534,11 +552,9 @@ Error validatePointerAllocaAccesses(AllocaInst &Alloca, Function &F) {
     bool UnsupportedAccess = false;
     if (auto *Load = dyn_cast<LoadInst>(&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<StoreInst>(&I)) {

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.

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"}

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"}

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<O2>", "-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()

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) {