[llvm] [SPIR-V] Ensure that a correct pointer type is deduced from the Value argument of OpAtomic* instructions (PR #127492)
Vyacheslav Levytskyy via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 17 05:34:58 PST 2025
https://github.com/VyacheslavLevytskyy created https://github.com/llvm/llvm-project/pull/127492
This PR improves the set of rules for type inference by ensuring that a correct pointer type is deduced from the Value argument of OpAtomic* instructions, also when a pointer argument is coming from an `inttoptr .. to` instruction that caused problems earlier. Existing test cases are updated accordingly. This fixes https://github.com/llvm/llvm-project/issues/127491
>From 707272706b90b821a19c769070f48c62c7a8b817 Mon Sep 17 00:00:00 2001
From: "Levytskyy, Vyacheslav" <vyacheslav.levytskyy at intel.com>
Date: Mon, 17 Feb 2025 05:25:21 -0800
Subject: [PATCH] Ensure that a correct pointer type is deduced from the Value
argument of OpAtomic* instructions
---
llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 37 ++++++++++++----
.../atomicrmw_faddfsub_float.ll | 33 +++++++++++++--
.../SPIRV/transcoding/atomic_load_store.ll | 42 +++++++++++++++++--
3 files changed, 99 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 251bc17fef52a..961e23e19f7b0 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -182,7 +182,7 @@ class SPIRVEmitIntrinsics
bool deduceOperandElementTypeCalledFunction(
CallInst *CI, SmallVector<std::pair<Value *, unsigned>> &Ops,
- Type *&KnownElemTy);
+ Type *&KnownElemTy, bool &Uncomplete);
void deduceOperandElementTypeFunctionPointer(
CallInst *CI, SmallVector<std::pair<Value *, unsigned>> &Ops,
Type *&KnownElemTy, bool IsPostprocessing);
@@ -893,7 +893,7 @@ static inline Type *getAtomicElemTy(SPIRVGlobalRegistry *GR, Instruction *I,
// indirect function invocation, and true otherwise.
bool SPIRVEmitIntrinsics::deduceOperandElementTypeCalledFunction(
CallInst *CI, SmallVector<std::pair<Value *, unsigned>> &Ops,
- Type *&KnownElemTy) {
+ Type *&KnownElemTy, bool &Uncomplete) {
Function *CalledF = CI->getCalledFunction();
if (!CalledF)
return false;
@@ -915,12 +915,15 @@ bool SPIRVEmitIntrinsics::deduceOperandElementTypeCalledFunction(
Ops.push_back(std::make_pair(Op, i));
}
} else if (Grp == SPIRV::Atomic || Grp == SPIRV::AtomicFloating) {
- if (CI->arg_size() < 2)
+ if (CI->arg_size() == 0)
return true;
Value *Op = CI->getArgOperand(0);
if (!isPointerTy(Op->getType()))
return true;
switch (Opcode) {
+ case SPIRV::OpAtomicFAddEXT:
+ case SPIRV::OpAtomicFMinEXT:
+ case SPIRV::OpAtomicFMaxEXT:
case SPIRV::OpAtomicLoad:
case SPIRV::OpAtomicCompareExchangeWeak:
case SPIRV::OpAtomicCompareExchange:
@@ -934,9 +937,23 @@ bool SPIRVEmitIntrinsics::deduceOperandElementTypeCalledFunction(
case SPIRV::OpAtomicUMax:
case SPIRV::OpAtomicSMin:
case SPIRV::OpAtomicSMax: {
- KnownElemTy = getAtomicElemTy(GR, CI, Op);
+ KnownElemTy = isPointerTy(CI->getType()) ? getAtomicElemTy(GR, CI, Op)
+ : CI->getType();
if (!KnownElemTy)
return true;
+ Uncomplete = isTodoType(Op);
+ Ops.push_back(std::make_pair(Op, 0));
+ } break;
+ case SPIRV::OpAtomicStore: {
+ if (CI->arg_size() < 4)
+ return true;
+ Value *ValOp = CI->getArgOperand(3);
+ KnownElemTy = isPointerTy(ValOp->getType())
+ ? getAtomicElemTy(GR, CI, Op)
+ : ValOp->getType();
+ if (!KnownElemTy)
+ return true;
+ Uncomplete = isTodoType(Op);
Ops.push_back(std::make_pair(Op, 0));
} break;
}
@@ -1090,15 +1107,21 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
Ops.push_back(std::make_pair(Ref->getPointerOperand(),
StoreInst::getPointerOperandIndex()));
} else if (auto *Ref = dyn_cast<AtomicCmpXchgInst>(I)) {
- KnownElemTy = getAtomicElemTy(GR, I, Ref->getPointerOperand());
+ KnownElemTy = isPointerTy(I->getType())
+ ? getAtomicElemTy(GR, I, Ref->getPointerOperand())
+ : I->getType();
if (!KnownElemTy)
return;
+ Uncomplete = isTodoType(Ref->getPointerOperand());
Ops.push_back(std::make_pair(Ref->getPointerOperand(),
AtomicCmpXchgInst::getPointerOperandIndex()));
} else if (auto *Ref = dyn_cast<AtomicRMWInst>(I)) {
- KnownElemTy = getAtomicElemTy(GR, I, Ref->getPointerOperand());
+ KnownElemTy = isPointerTy(I->getType())
+ ? getAtomicElemTy(GR, I, Ref->getPointerOperand())
+ : I->getType();
if (!KnownElemTy)
return;
+ Uncomplete = isTodoType(Ref->getPointerOperand());
Ops.push_back(std::make_pair(Ref->getPointerOperand(),
AtomicRMWInst::getPointerOperandIndex()));
} else if (auto *Ref = dyn_cast<SelectInst>(I)) {
@@ -1141,7 +1164,7 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
}
} else if (CallInst *CI = dyn_cast<CallInst>(I)) {
if (!CI->isIndirectCall())
- deduceOperandElementTypeCalledFunction(CI, Ops, KnownElemTy);
+ deduceOperandElementTypeCalledFunction(CI, Ops, KnownElemTy, Uncomplete);
else if (HaveFunPtrs)
deduceOperandElementTypeFunctionPointer(CI, Ops, KnownElemTy,
IsPostprocessing);
diff --git a/llvm/test/CodeGen/SPIRV/extensions/SPV_EXT_shader_atomic_float_add/atomicrmw_faddfsub_float.ll b/llvm/test/CodeGen/SPIRV/extensions/SPV_EXT_shader_atomic_float_add/atomicrmw_faddfsub_float.ll
index 075e63ea6de61..c6c8afc47dee3 100644
--- a/llvm/test/CodeGen/SPIRV/extensions/SPV_EXT_shader_atomic_float_add/atomicrmw_faddfsub_float.ll
+++ b/llvm/test/CodeGen/SPIRV/extensions/SPV_EXT_shader_atomic_float_add/atomicrmw_faddfsub_float.ll
@@ -1,6 +1,10 @@
; RUN: not llc -O0 -mtriple=spirv32-unknown-unknown %s -o %t.spvt 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown --spirv-ext=+SPV_EXT_shader_atomic_float_add %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown --spirv-ext=+SPV_EXT_shader_atomic_float_add %s -o - -filetype=obj | spirv-val %}
+
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_EXT_shader_atomic_float_add %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_EXT_shader_atomic_float_add %s -o - -filetype=obj | spirv-val %}
; CHECK-ERROR: LLVM ERROR: The atomic float instruction requires the following SPIR-V extension: SPV_EXT_shader_atomic_float_add
@@ -25,9 +29,6 @@
; CHECK: %[[Neg42:[0-9]+]] = OpFNegate %[[TyFP32]] %[[Const42]]
; CHECK: OpAtomicFAddEXT %[[TyFP32]] %[[DblPtr]] %[[ScopeWorkgroup]] %[[WorkgroupMemory]] %[[Neg42]]
-target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
-target triple = "spir64"
-
@f = common dso_local local_unnamed_addr addrspace(1) global float 0.000000e+00, align 8
define dso_local spir_func void @test1() local_unnamed_addr {
@@ -55,5 +56,31 @@ entry:
declare spir_func float @_Z25atomic_fetch_add_explicitPU3AS1VU7_Atomicff12memory_order(ptr addrspace(1), float, i32)
declare spir_func float @_Z25atomic_fetch_sub_explicitPU3AS1VU7_Atomicff12memory_order(ptr addrspace(1), float, i32)
+; CHECK: %[[#Ptr1:]] = OpConvertUToPtr %[[TyFP32Ptr]] %[[#]]
+; CHECK: %[[#]] = OpAtomicFAddEXT %[[TyFP32]] %[[#Ptr1]] %[[#]] %[[#]] %[[#]]
+; CHECK: %[[#Ptr2:]] = OpConvertUToPtr %[[TyFP32Ptr]] %[[#]]
+; CHECK: %[[#]] = OpAtomicFAddEXT %[[TyFP32]] %[[#Ptr2]] %[[#]] %[[#]] %[[#]]
+; CHECK: %[[#Ptr3:]] = OpConvertUToPtr %[[TyFP32Ptr]] %[[#]]
+; CHECK: %[[#]] = OpAtomicFAddEXT %[[TyFP32]] %[[#Ptr3]] %[[#]] %[[#]] %[[#]]
+; CHECK: %[[#Ptr4:]] = OpConvertUToPtr %[[TyFP32Ptr]] %[[#]]
+; CHECK: %[[#]] = OpAtomicFAddEXT %[[TyFP32]] %[[#Ptr4]] %[[#]] %[[#]] %[[#]]
+; CHECK: %[[#Ptr5:]] = OpConvertUToPtr %[[TyFP32Ptr]] %[[#]]
+; CHECK: %[[#]] = OpAtomicFAddEXT %[[TyFP32]] %[[#Ptr5]] %[[#]] %[[#]] %[[#]]
+
+define dso_local spir_func void @test4(i64 noundef %arg, float %val) local_unnamed_addr {
+entry:
+ %ptr1 = inttoptr i64 %arg to float addrspace(1)*
+ %v1 = atomicrmw fadd ptr addrspace(1) %ptr1, float %val seq_cst, align 4
+ %ptr2 = inttoptr i64 %arg to float addrspace(1)*
+ %v2 = atomicrmw fsub ptr addrspace(1) %ptr2, float %val seq_cst, align 4
+ %ptr3 = inttoptr i64 %arg to float addrspace(1)*
+ %v3 = tail call spir_func float @_Z21__spirv_AtomicFAddEXT(ptr addrspace(1) %ptr3, i32 1, i32 16, float %val)
+ %ptr4 = inttoptr i64 %arg to float addrspace(1)*
+ %v4 = tail call spir_func float @_Z25atomic_fetch_add_explicitPU3AS1VU7_Atomicff12memory_order(ptr addrspace(1) %ptr4, float %val, i32 0)
+ %ptr5 = inttoptr i64 %arg to float addrspace(1)*
+ %v5 = tail call spir_func float @_Z25atomic_fetch_sub_explicitPU3AS1VU7_Atomicff12memory_order(ptr addrspace(1) %ptr5, float %val, i32 0)
+ ret void
+}
+
!llvm.module.flags = !{!0}
!0 = !{i32 1, !"wchar_size", i32 4}
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/atomic_load_store.ll b/llvm/test/CodeGen/SPIRV/transcoding/atomic_load_store.ll
index 3e5a3ac356936..17a915e33c973 100644
--- a/llvm/test/CodeGen/SPIRV/transcoding/atomic_load_store.ll
+++ b/llvm/test/CodeGen/SPIRV/transcoding/atomic_load_store.ll
@@ -1,6 +1,9 @@
; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
;; Check 'LLVM ==> SPIR-V' conversion of atomic_load and atomic_store.
; CHECK-SPIRV-LABEL: OpFunction
@@ -17,17 +20,50 @@ entry:
; CHECK-SPIRV-LABEL: OpFunction
; CHECK-SPIRV-NEXT: %[[#object:]] = OpFunctionParameter %[[#]]
-; CHECK-SPIRV-NEXT: OpFunctionParameter
; CHECK-SPIRV-NEXT: %[[#desired:]] = OpFunctionParameter %[[#]]
; CHECK-SPIRV: OpAtomicStore %[[#object]] %[[#]] %[[#]] %[[#desired]]
; CHECK-SPIRV-LABEL: OpFunctionEnd
-define spir_func void @test_store(i32 addrspace(4)* %object, i32 addrspace(4)* %expected, i32 %desired) {
+define spir_func void @test_store(i32 addrspace(4)* %object, i32 %desired) {
entry:
call spir_func void @_Z12atomic_storePVU3AS4U7_Atomicii(i32 addrspace(4)* %object, i32 %desired)
ret void
}
declare spir_func i32 @_Z11atomic_loadPVU3AS4U7_Atomici(i32 addrspace(4)*)
-
declare spir_func void @_Z12atomic_storePVU3AS4U7_Atomicii(i32 addrspace(4)*, i32)
+
+; The goal of @test_typesX() cases is to ensure that a correct pointer type
+; is deduced from the Value argument of OpAtomicLoad/OpAtomicStore. There is
+; no need to add more pattern matching rules to be sure that the pointer type
+; is valid, it's enough that `spirv-val` considers the output valid as it
+; checks the same condition while validating the output.
+
+define spir_func void @test_types1(ptr addrspace(1) %ptr, float %val) {
+entry:
+ %r = call spir_func float @atomic_load(ptr addrspace(1) %ptr)
+ ret void
+}
+
+define spir_func void @test_types2(ptr addrspace(1) %ptr, float %val) {
+entry:
+ call spir_func void @atomic_store(ptr addrspace(1) %ptr, float %val)
+ ret void
+}
+
+define spir_func void @test_types3(i64 noundef %arg, float %val) {
+entry:
+ %ptr1 = inttoptr i64 %arg to float addrspace(1)*
+ %r = call spir_func float @atomic_load(ptr addrspace(1) %ptr1)
+ ret void
+}
+
+define spir_func void @test_types4(i64 noundef %arg, float %val) {
+entry:
+ %ptr2 = inttoptr i64 %arg to float addrspace(1)*
+ call spir_func void @atomic_store(ptr addrspace(1) %ptr2, float %val)
+ ret void
+}
+
+declare spir_func float @atomic_load(ptr addrspace(1))
+declare spir_func void @atomic_store(ptr addrspace(1), float)
More information about the llvm-commits
mailing list