Fix reverse translation of non-overloaded AMDGCN pointer intrinsics

[idubinov]

The similar changes to PR246, but for reverse translation.
Fixes CI ROCm Examples / HIP Basic test
More info provided: (#271)

Reflecting #186, removed as much as I think related to constant address space etc. Now the types are resolves on the reader side based on .td files instead of duct tape solutions we had previously.

I want to ask to take a look, I may miss something or misunderstand

Summary

Non-overloaded AMDGCN intrinsics that return a pointer in a fixed address space — e.g. llvm.amdgcn.implicitarg.ptr, which must return ptr addrspace(4) — are lowered to a spirv.llvm_* imported function during forward translation. On reverse translation, SPIRVToLLVM::transFunction recreated them with Function::Create() using the SPIR-V–derived function type. SPIR-V has no notion of the AMDGPU-specific address spaces, so the return pointer falls back to addrspace(0), producing a declaration that mismatches the intrinsic's required prototype and fails the IR verifier:

intrinsic return type expected ptr addrspace(4), but got ptr

memcpy / ptrmask / umul_with_overflow already have per-intrinsic special cases just above. Instead of adding one per intrinsic, this reconstructs any non-overloaded intrinsic from its canonical declaration via Intrinsic::getOrInsertDeclaration, which yields the correct, verifier-valid prototype. Overloaded intrinsics still fall through to the existing type-derived handling (they need overload types from the SPIR-V function type).

Impact

This surfaces at runtime when finalizing amdgcnspirv HIP code objects. With recent HIP headers (clr SWDEV-548892 / #2439), threadIdx/blockIdx/blockDim lower to llvm.amdgcn.workitem.id / workgroup.id / implicitarg.ptr. The broken reverse-translated module is fed to comgr's in-process clang codegen at first kernel launch, which crashes (SIGSEGV in the broken-module teardown path). This reproduces as HIP-Basic rocm-examples segfaults in the Test rocm-examples CI job (16/23 examples).

Verification (local, gfx90a + amd-staging clang-24 toolchain)

• Built the full CI toolchain (amd-staging llvm-project + this translator + comgr + ROCr/CLR) and HIP-Basic with -DCMAKE_HIP_ARCHITECTURES=amdgcnspirv.
• Before: 14/21 examples SIGSEGV at first kernel launch.
• After: ctest 21/21 pass.
• Minimal repro: amd-llvm-spirv -r on real HIP SPIR-V no longer emits the broken declare ptr @llvm.amdgcn.implicitarg.ptr(); opt -passes=verify is clean.
• Translator lit check-amd-llvm-spirv: no new failures (the 4 pre-existing DebugInfo/* failures are unrelated — they fail identically without this change).

Notes

• Draft: opening to run CI. A reduced reverse-translation regression test (a .spt carrying the regularized spirv.llvm_amdgcn_implicitarg_ptr import) is prepared but omitted here because its raw SPIR-V trips push secret-scanning on an embedded __hip_cuid_* string; happy to add it (redacted) if wanted.

[aobolensk]

Any test for this case?

[aobolensk]

For me it looks like a more general problem of some sort. Do you know why other intrinsics do not require getOrInsertDeclaration but AMDGCN ones do?

[idubinov]

Most LLVM intrinsics are "known" to the translator: they map to real SPIR-V opcodes/ExtInsts and are reconstructed on reverse from that semantic representation, so their type is never in doubt. AMDGCN intrinsics have no SPIR-V equivalent, so they travel as opaque -spirv-allow-unknown-intrinsics passthroughs — only a mangled name plus a SPIR-V function type survive, and since SPIR-V encodes pointers by storage class, the AMDGPU address spaces (e.g. the constant AS(4) that implicitarg.ptr must return) get lost when the type is rebuilt. Because they're still named LLVM intrinsics, the verifier enforces their exact prototype - so we have to recover it from the intrinsic definition via getOrInsertDeclaration rather than trust the address-space-lossy reconstructed type.

Intrinsic::getOrInsertDeclaration(M, IID) builds the declaration from the intrinsic's authoritative .td definition, so it sets the entire prototype at once - including the correct return type with its required address space (ptr addrspace(4)) - not just the name.

[idubinov]

Test added

[aobolensk]

If we're supposed to mimic the patch #246 with removal ugly WA related to "llvm.amdgcn.is.shared" and "llvm.amdgcn.is.private", should we remove/update this part

SPIRV-LLVM-Translator/lib/SPIRV/SPIRVReader.cpp

Lines 2947 to 2959 in 82000fe

	} else if (Args.size() == 1 &&
	(BC->getFunction()->getName() == "llvm.amdgcn.is.shared" ||
	BC->getFunction()->getName() == "llvm.amdgcn.is.private")) {
	if (BC->getArgumentValues().front()->getType()->getPointerStorageClass()
	!= StorageClassGeneric) {
	auto *PTy = PointerType::get(
	F->getContext(), mapSPIRVAddrSpaceToAMDGPU(StorageClassGeneric));
	Args[0] =
	CastInst::CreatePointerBitCastOrAddrSpaceCast(Args[0], PTy, "",
	BB);
	}
	}
	}

anyhow?

[idubinov]

Removed everything i have found related to duplicated logic for recovering address spaces and etc. Now types are recovered from .td files

[idubinov]

due to problems with CI machine, the ROCm Examples tests passed on local machine gfx90a:

HIP-Basic    │ 23/23
Applications │ 7/7 
Libraries    │ 16/16

[aobolensk]

That looks much better, because we're removing the difference with the upstream translator