Skip to content

Commit ffdcbc0

Browse files
aratajewigcbot
authored andcommitted
Fix crash in ProgramScopeConstantAnalysis during recompilation
This change fixes crashes in `ProgramScopeConstantAnalysis` during recompilation. Here is an example LLVM IR that leads to the crash: ```llvm @0 = internal unnamed_addr addrspace(2) constant [24 x i8] zeroinitializer, align 4 @1 = internal unnamed_addr addrspace(2) constant [48 x i8] zeroinitializer, align 4 @2 = internal unnamed_addr addrspace(2) constant [24 x i8] zeroinitializer, align 4 define spir_kernel void @kernel0() { ... %ptr0 = bitcast [24 x i8] addrspace(2)* @0 to i8 addrspace(2)* call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 4 %dst0, i8 addrspace(2)* align 4 %ptr, i64 24, i1 false) ... %ptr2 = bitcast [24 x i8] addrspace(2)* @3 to i8 addrspace(2)* call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 4 %dst2, i8 addrspace(2)* align 4 %ptr2, i64 24, i1 false) ... } define spir_kernel void @kernel1() { ... %ptr1 = bitcast [48 x i8] addrspace(2)* @1 to i8 addrspace(2)* call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 4 %dst2, i8 addrspace(2)* align 4 %ptr1, i64 48, i1 false) ... } ``` During first compilation (RetryManager's `stateId == 0`) - `ProgramScopeConstantAnalysis` pass assigns names to global variables: ```llvm @gVar.0 = internal unnamed_addr addrspace(2) constant [24 x i8] zeroinitializer, align 4 @gVar.1 = internal unnamed_addr addrspace(2) constant [48 x i8] zeroinitializer, align 4 @gVar.2 = internal unnamed_addr addrspace(2) constant [24 x i8] zeroinitializer, align 4 define spir_kernel void @kernel0() { ... %ptr0 = bitcast [24 x i8] addrspace(2)* @gVar.0 to i8 addrspace(2)* call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 4 %dst0, i8 addrspace(2)* align 4 %ptr, i64 24, i1 false) ... %ptr2 = bitcast [24 x i8] addrspace(2)* @gVar.2 to i8 addrspace(2)* call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 4 %dst2, i8 addrspace(2)* align 4 %ptr2, i64 24, i1 false) ... } define spir_kernel void @kernel1() { ... %ptr1 = bitcast [48 x i8] addrspace(2)* @gVar.1 to i8 addrspace(2)* call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 4 %dst2, i8 addrspace(2)* align 4 %ptr1, i64 48, i1 false) ... } ``` During the second compilation ((RetryManager's `stateId == 1`) ), note that `kernel1` is removed as it does not require recompilation. Consequently, the original `@1` global variable is also removed since it was exclusively utilized by `@kernel1`. As a result, `@2` is renamed to `@gVar.1`, whereas it was previously named `@gVar.2` in the initial compilation. This renaming can potentially lead to crashes or incorrect offset assignments in the `ProgramScopeConstantAnalysis` pass. ```llvm @gVar.0 = internal unnamed_addr addrspace(2) constant [24 x i8] zeroinitializer, align 4 @gVar.1 = internal unnamed_addr addrspace(2) constant [24 x i8] zeroinitializer, align 4 ; it was named `@gVar.2` during first compilation define spir_kernel void @kernel0() { ... %ptr0 = bitcast [24 x i8] addrspace(2)* @0 to i8 addrspace(2)* call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 4 %dst0, i8 addrspace(2)* align 4 %ptr, i64 24, i1 false) ... %ptr2 = bitcast [24 x i8] addrspace(2)* @3 to i8 addrspace(2)* call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 4 %dst2, i8 addrspace(2)* align 4 %ptr2, i64 24, i1 false) ... } ``` To prevent potential issues, this change ensures that unnamed global variables are assigned names right after SPIRV->LLVM translation, therefore prior to - removing kernels that do not require recompilation, and prior to - executing the DeadGlobalElimination pass.
1 parent 53c3db5 commit ffdcbc0

File tree

21 files changed

+18
-5392
lines changed

21 files changed

+18
-5392
lines changed

IGC/AdaptorOCL/dllInterfaceCompute.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,21 @@ void GenerateCompilerOptionsMD(
346346
NamedMD->addOperand(llvm::MDNode::get(C, ValueVec));
347347
}
348348

349+
// Ensure unnamed global variables are assigned names immediately after translating from SPIRV to LLVM.
350+
// This must occur before removing kernels that do not require recompilation.
351+
// Naming global variables after kernels removal can result in inconsistent naming compared to the first compilation,
352+
// potentially causing crashes in the ProgramScopeConstantAnalysis pass.
353+
void AssignNamesToUnnamedGlobalVariables(llvm::Module& M)
354+
{
355+
for (auto& G : M.getGlobalList())
356+
{
357+
if (!G.hasName())
358+
{
359+
G.setName("gVar");
360+
}
361+
}
362+
}
363+
349364
// Dump shader (binary or text), to output directory.
350365
// Create directory if it doesn't exist.
351366
// Works for all OSes.
@@ -493,6 +508,8 @@ bool TranslateSPIRVToLLVM(
493508
// Handle OpenCL Compiler Options
494509
if (success)
495510
{
511+
AssignNamesToUnnamedGlobalVariables(*LLVMModule);
512+
496513
GenerateCompilerOptionsMD(
497514
Context,
498515
*LLVMModule,

IGC/ocloc_tests/SPIRV-Asm/OpCopyMemorySized_alignment.spvasm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,6 @@
5757
; CHECK-NOT: call void @llvm.memcpy
5858
; CHECK: [[ALLOCA:%.*]] = alloca %struct.complex
5959
; CHECK: [[DST:%.*]] = bitcast %struct.complex* [[ALLOCA]] to i8*
60-
; CHECK: [[SRC:%.*]] = getelementptr inbounds [16 x i8], [16 x i8] addrspace(2)* @0, i32 0, i32 0
60+
; CHECK: [[SRC:%.*]] = getelementptr inbounds [16 x i8], [16 x i8] addrspace(2)* @{{.*}}, i32 0, i32 0
6161
; CHECK: call void @llvm.memcpy.p0i8.p2i8.i64(i8* align 8 [[DST]], i8 {{.*}}[[SRC]], i64 16, i1 false)
6262
; CHECK: ret void

IGC/ocloc_tests/SPIRV-extenstions/SPV_INTEL_fpga_buffer_location/FPGABufferLocation.ll

Lines changed: 0 additions & 36 deletions
This file was deleted.

IGC/ocloc_tests/SPIRV-extenstions/SPV_INTEL_fpga_cluster_attributes/function_attributes.ll

Lines changed: 0 additions & 25 deletions
This file was deleted.

IGC/ocloc_tests/SPIRV-extenstions/SPV_INTEL_fpga_dsp_control/prefer_dsp.ll

Lines changed: 0 additions & 84 deletions
This file was deleted.

IGC/ocloc_tests/SPIRV-extenstions/SPV_INTEL_fpga_dsp_control/prefer_dsp_propagate.ll

Lines changed: 0 additions & 86 deletions
This file was deleted.

0 commit comments

Comments
 (0)