Skip to content

Commit 9e02e8f

Browse files
fix producing multiple identical opaque pointer types (llvm#79060)
This PR fixes llvm#79057 and improves code generation for opaque pointers by replacing the culprit SPIRVGlobalRegistry::getOpTypePointer() call with a more appropriate SPIRVGlobalRegistry::getOrCreateSPIRVPointerType() call. The latter function works together with the `DuplicatesTracker` (`SPIRVGeneralDuplicatesTracker DT;` from `class SPIRVGlobalRegistry`) to trace existence of previous definitions of opaque pointers. This allows to produce just one `OpTypePointer` command for all identical opaque pointers definitions and to return the very same type record for subsequent `SPIRVGlobalRegistry::createSPIRVType()` invocations. This PR alone improves code generation by producing a single needed definition per all opaque pointers to i8 of the same address space instead of multiple identical definitions produced before the patch. From the root cause analysis of llvm#79057 we see also that this PR resolves the problem of inconsistency between keeping multiple instruction for identical opaque pointer types and just a single record for all such instructions in the `DuplicatesTracker`, and so it also resolves the issue with crashes on creation of a struct with opaque pointer fields due to the fact that now such struct fields refer to the same operand `<id>` having a required record in the data structure used for dependencies analysis (see llvm#79057).
1 parent 3948379 commit 9e02e8f

File tree

3 files changed

+21
-6
lines changed

3 files changed

+21
-6
lines changed

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -716,13 +716,14 @@ SPIRVType *SPIRVGlobalRegistry::createSPIRVType(
716716
ForwardPointerTypes[PType] = getOpTypeForwardPointer(SC, MIRBuilder);
717717
return ForwardPointerTypes[PType];
718718
}
719-
Register Reg(0);
720719
// If we have forward pointer associated with this type, use its register
721720
// operand to create OpTypePointer.
722-
if (ForwardPointerTypes.contains(PType))
723-
Reg = getSPIRVTypeID(ForwardPointerTypes[PType]);
721+
if (ForwardPointerTypes.contains(PType)) {
722+
Register Reg = getSPIRVTypeID(ForwardPointerTypes[PType]);
723+
return getOpTypePointer(SC, SpvElementType, MIRBuilder, Reg);
724+
}
724725

725-
return getOpTypePointer(SC, SpvElementType, MIRBuilder, Reg);
726+
return getOrCreateSPIRVPointerType(SpvElementType, MIRBuilder, SC);
726727
}
727728
llvm_unreachable("Unable to convert LLVM type to SPIRVType");
728729
}

llvm/test/CodeGen/SPIRV/opencl/device_execution/execute_block.ll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
22

3-
; TODO(#60133): Requires updates following opaque pointer migration.
4-
; XFAIL: *
53
; REQUIRES: asserts
64

75
; CHECK: %[[#bool:]] = OpTypeBool
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
2+
; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
3+
4+
; CHECK: %[[TyInt8:.*]] = OpTypeInt 8 0
5+
; CHECK: %[[TyInt8Ptr:.*]] = OpTypePointer {{[a-zA-Z]+}} %[[TyInt8]]
6+
; CHECK: %[[TyStruct:.*]] = OpTypeStruct %[[TyInt8Ptr]] %[[TyInt8Ptr]]
7+
; CHECK: %[[ConstStruct:.*]] = OpConstantComposite %[[TyStruct]] %[[ConstField:.*]] %[[ConstField]]
8+
; CHECK: %[[TyStructPtr:.*]] = OpTypePointer {{[a-zA-Z]+}} %[[TyStruct]]
9+
; CHECK: OpVariable %[[TyStructPtr]] {{[a-zA-Z]+}} %[[ConstStruct]]
10+
11+
@a = addrspace(1) constant i32 123
12+
@struct = addrspace(1) global {ptr addrspace(1), ptr addrspace(1)} { ptr addrspace(1) @a, ptr addrspace(1) @a }
13+
14+
define spir_kernel void @foo() {
15+
ret void
16+
}

0 commit comments

Comments
 (0)