Skip to content

Commit a705167

Browse files
Both GetElementPtrConstantExpr and GetElementPtrInst may represent access to a buildin variable (#2487)
This PR is to fix the issue when SPIRV Translator expects now to see exactly GetElementPtrInst in an access chain for a builtin variable and crashes in lib/SPIRV/SPIRVUtil.cpp in replaceUsesOfBuiltinVar() with llvm_unreachable() call. See #2486 for the detailed description. Attached is a test case that demonstrates the problem and leads to a crash in current version(s) of SPIRV Translator.
1 parent 2c3b505 commit a705167

File tree

2 files changed

+59
-2
lines changed

2 files changed

+59
-2
lines changed

lib/SPIRV/SPIRVUtil.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
#include "llvm/IR/IRBuilder.h"
5858
#include "llvm/IR/IntrinsicInst.h"
5959
#include "llvm/IR/Metadata.h"
60+
#include "llvm/IR/Operator.h"
6061
#include "llvm/IR/TypedPointerType.h"
6162
#include "llvm/Support/CommandLine.h"
6263
#include "llvm/Support/Debug.h"
@@ -1978,13 +1979,14 @@ static void replaceUsesOfBuiltinVar(Value *V, const APInt &AccumulatedOffset,
19781979
if (auto *Cast = dyn_cast<CastInst>(U)) {
19791980
replaceUsesOfBuiltinVar(Cast, AccumulatedOffset, ReplacementFunc, GV);
19801981
InstsToRemove.push_back(Cast);
1981-
} else if (auto *GEP = dyn_cast<GetElementPtrInst>(U)) {
1982+
} else if (auto *GEP = dyn_cast<GEPOperator>(U)) {
19821983
APInt NewOffset = AccumulatedOffset.sextOrTrunc(
19831984
DL.getIndexSizeInBits(GEP->getPointerAddressSpace()));
19841985
if (!GEP->accumulateConstantOffset(DL, NewOffset))
19851986
llvm_unreachable("Illegal GEP of a SPIR-V builtin variable");
19861987
replaceUsesOfBuiltinVar(GEP, NewOffset, ReplacementFunc, GV);
1987-
InstsToRemove.push_back(GEP);
1988+
if (auto *AsInst = dyn_cast<Instruction>(U))
1989+
InstsToRemove.push_back(AsInst);
19881990
} else if (auto *Load = dyn_cast<LoadInst>(U)) {
19891991
// Figure out which index the accumulated offset corresponds to. If we
19901992
// have a weird offset (e.g., trying to load byte 7), bail out.

test/GEPOperator.spvasm

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
; It's possible that access to builtin variable is represented by
2+
; GetElementPtrConstantExpr rather than GetElementPtrInst. This
3+
; test case presents a pattern that results into a ConstantExpr.
4+
5+
; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
6+
; RUN: spirv-val %t.spv
7+
; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s
8+
9+
OpCapability Kernel
10+
OpCapability Addresses
11+
OpCapability Int64
12+
OpCapability Int8
13+
OpCapability GenericPointer
14+
OpCapability Linkage
15+
OpMemoryModel Physical64 OpenCL
16+
OpEntryPoint Kernel %foo "test"
17+
OpDecorate %__spirv_BuiltInWorkgroupSize Constant
18+
OpDecorate %__spirv_BuiltInWorkgroupSize Alignment 32
19+
OpDecorate %__spirv_BuiltInWorkgroupSize LinkageAttributes "__spirv_BuiltInWorkgroupSize" Import
20+
OpDecorate %__spirv_BuiltInWorkgroupSize BuiltIn WorkgroupSize
21+
%void = OpTypeVoid
22+
%ulong = OpTypeInt 64 0
23+
%uint = OpTypeInt 32 0
24+
%uchar = OpTypeInt 8 0
25+
%v3ulong = OpTypeVector %ulong 3
26+
%_ptr_CW_uchar = OpTypePointer CrossWorkgroup %uchar
27+
%_ptr_CW_v3ulong = OpTypePointer CrossWorkgroup %v3ulong
28+
%_ptr_CW_ulong = OpTypePointer CrossWorkgroup %ulong
29+
%foo_type = OpTypeFunction %void
30+
%uint_0 = OpConstant %uint 0
31+
%ulong_8 = OpConstant %ulong 8
32+
%ulong_16 = OpConstant %ulong 16
33+
%__spirv_BuiltInWorkgroupSize = OpVariable %_ptr_CW_v3ulong CrossWorkgroup
34+
%c1 = OpSpecConstantOp %_ptr_CW_uchar InBoundsPtrAccessChain %__spirv_BuiltInWorkgroupSize %uint_0 %ulong_8
35+
%c2 = OpSpecConstantOp %_ptr_CW_uchar InBoundsPtrAccessChain %__spirv_BuiltInWorkgroupSize %uint_0 %ulong_16
36+
%foo = OpFunction %void None %foo_type
37+
%entry = OpLabel
38+
%pv0 = OpBitcast %_ptr_CW_ulong %__spirv_BuiltInWorkgroupSize
39+
%v0 = OpLoad %ulong %pv0 Aligned 32
40+
%pv1 = OpBitcast %_ptr_CW_ulong %c1
41+
%v1 = OpLoad %ulong %pv1 Aligned 8
42+
%idx1 = OpIMul %ulong %v0 %v1
43+
%pv2 = OpBitcast %_ptr_CW_ulong %c2
44+
%v2 = OpLoad %ulong %pv2 Aligned 16
45+
%idx2 = OpIMul %ulong %idx1 %v2
46+
OpReturn
47+
OpFunctionEnd
48+
49+
; CHECK-NOT: getelementptr
50+
; CHECK-NOT: load
51+
; CHECK: %[[#V0:]] = call spir_func i64 @_Z14get_local_sizej(i32 0)
52+
; CHECK: %[[#V1:]] = call spir_func i64 @_Z14get_local_sizej(i32 8)
53+
; CHECK: %[[#Idx1:]] = mul i64 %[[#V0]], %[[#V1]]
54+
; CHECK: %[[#V2:]] = call spir_func i64 @_Z14get_local_sizej(i32 16)
55+
; CHECK: %[[#Idx2:]] = mul i64 %[[#Idx1]], %[[#V2]]

0 commit comments

Comments
 (0)