Skip to content

Commit

Permalink
Both GetElementPtrConstantExpr and GetElementPtrInst may represent ac…
Browse files Browse the repository at this point in the history
…cess 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.

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@a7051670770f471
  • Loading branch information
VyacheslavLevytskyy authored and sys-ce-bb committed Apr 11, 2024
1 parent 3caac02 commit b89f46f
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 2 deletions.
6 changes: 4 additions & 2 deletions llvm-spirv/lib/SPIRV/SPIRVUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Operator.h"
#include "llvm/IR/TypedPointerType.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
Expand Down Expand Up @@ -1980,13 +1981,14 @@ static void replaceUsesOfBuiltinVar(Value *V, const APInt &AccumulatedOffset,
if (auto *Cast = dyn_cast<CastInst>(U)) {
replaceUsesOfBuiltinVar(Cast, AccumulatedOffset, ReplacementFunc, GV);
InstsToRemove.push_back(Cast);
} else if (auto *GEP = dyn_cast<GetElementPtrInst>(U)) {
} else if (auto *GEP = dyn_cast<GEPOperator>(U)) {
APInt NewOffset = AccumulatedOffset.sextOrTrunc(
DL.getIndexSizeInBits(GEP->getPointerAddressSpace()));
if (!GEP->accumulateConstantOffset(DL, NewOffset))
llvm_unreachable("Illegal GEP of a SPIR-V builtin variable");
replaceUsesOfBuiltinVar(GEP, NewOffset, ReplacementFunc, GV);
InstsToRemove.push_back(GEP);
if (auto *AsInst = dyn_cast<Instruction>(U))
InstsToRemove.push_back(AsInst);
} else if (auto *Load = dyn_cast<LoadInst>(U)) {
// Figure out which index the accumulated offset corresponds to. If we
// have a weird offset (e.g., trying to load byte 7), bail out.
Expand Down
55 changes: 55 additions & 0 deletions llvm-spirv/test/GEPOperator.spvasm
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
; It's possible that access to builtin variable is represented by
; GetElementPtrConstantExpr rather than GetElementPtrInst. This
; test case presents a pattern that results into a ConstantExpr.

; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
; RUN: spirv-val %t.spv
; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s

OpCapability Kernel
OpCapability Addresses
OpCapability Int64
OpCapability Int8
OpCapability GenericPointer
OpCapability Linkage
OpMemoryModel Physical64 OpenCL
OpEntryPoint Kernel %foo "test"
OpDecorate %__spirv_BuiltInWorkgroupSize Constant
OpDecorate %__spirv_BuiltInWorkgroupSize Alignment 32
OpDecorate %__spirv_BuiltInWorkgroupSize LinkageAttributes "__spirv_BuiltInWorkgroupSize" Import
OpDecorate %__spirv_BuiltInWorkgroupSize BuiltIn WorkgroupSize
%void = OpTypeVoid
%ulong = OpTypeInt 64 0
%uint = OpTypeInt 32 0
%uchar = OpTypeInt 8 0
%v3ulong = OpTypeVector %ulong 3
%_ptr_CW_uchar = OpTypePointer CrossWorkgroup %uchar
%_ptr_CW_v3ulong = OpTypePointer CrossWorkgroup %v3ulong
%_ptr_CW_ulong = OpTypePointer CrossWorkgroup %ulong
%foo_type = OpTypeFunction %void
%uint_0 = OpConstant %uint 0
%ulong_8 = OpConstant %ulong 8
%ulong_16 = OpConstant %ulong 16
%__spirv_BuiltInWorkgroupSize = OpVariable %_ptr_CW_v3ulong CrossWorkgroup
%c1 = OpSpecConstantOp %_ptr_CW_uchar InBoundsPtrAccessChain %__spirv_BuiltInWorkgroupSize %uint_0 %ulong_8
%c2 = OpSpecConstantOp %_ptr_CW_uchar InBoundsPtrAccessChain %__spirv_BuiltInWorkgroupSize %uint_0 %ulong_16
%foo = OpFunction %void None %foo_type
%entry = OpLabel
%pv0 = OpBitcast %_ptr_CW_ulong %__spirv_BuiltInWorkgroupSize
%v0 = OpLoad %ulong %pv0 Aligned 32
%pv1 = OpBitcast %_ptr_CW_ulong %c1
%v1 = OpLoad %ulong %pv1 Aligned 8
%idx1 = OpIMul %ulong %v0 %v1
%pv2 = OpBitcast %_ptr_CW_ulong %c2
%v2 = OpLoad %ulong %pv2 Aligned 16
%idx2 = OpIMul %ulong %idx1 %v2
OpReturn
OpFunctionEnd

; CHECK-NOT: getelementptr
; CHECK-NOT: load
; CHECK: %[[#V0:]] = call spir_func i64 @_Z14get_local_sizej(i32 0)
; CHECK: %[[#V1:]] = call spir_func i64 @_Z14get_local_sizej(i32 8)
; CHECK: %[[#Idx1:]] = mul i64 %[[#V0]], %[[#V1]]
; CHECK: %[[#V2:]] = call spir_func i64 @_Z14get_local_sizej(i32 16)
; CHECK: %[[#Idx2:]] = mul i64 %[[#Idx1]], %[[#V2]]

0 comments on commit b89f46f

Please sign in to comment.