Skip to content

Commit 961a7ce

Browse files
michalpaszkowskiigcbot
authored andcommitted
Support workaround __spirv_ConvertPtrToU with TargetExtTy and improve BTI arg rewriting
Workaround a SPIR-V/LLVM-Translator bug where images under opaque pointers lead to illegal ConvertPtrToU and SPIR-V Reader emits "__spirv_ConvertPtrToU". This produced bad call graph edges and crashes on TargetExtTy ptrtoint. In BTIAssignment, replace i32 BTI args directly, fold stores to the computed i32, and replace "__spirv_ConvertPtrToU" with the i32 BTI constant.
1 parent 1a633a4 commit 961a7ce

File tree

2 files changed

+115
-14
lines changed

2 files changed

+115
-14
lines changed

IGC/VectorCompiler/lib/GenXOpts/CMTrans/CMImpParam.cpp

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,18 +1042,47 @@ template <bool IsEntry> void CallGraphTraverser::visitFunction(Function &F) {
10421042
// Skipping reference edges.
10431043
if (!CallEdge.first)
10441044
continue;
1045-
Value *CI = IGCLLVM::makeOptional(CallEdge.first).value();
1045+
CallInst *CI =
1046+
dyn_cast<CallInst>(IGCLLVM::makeOptional(CallEdge.first).value());
10461047
// Skipping inline asm.
1047-
if (isa<CallInst>(CI) && cast<CallInst>(CI)->isInlineAsm())
1048+
if (CI && CI->isInlineAsm())
10481049
continue;
1049-
if (!isa<CallInst>(CI) || vc::isAnyNonTrivialIntrinsic(CI))
1050+
1051+
if (!CI || vc::isAnyNonTrivialIntrinsic(CI))
1052+
continue;
1053+
1054+
#if LLVM_VERSION_MAJOR >= 16
1055+
// This is a workaround due to a bug in Khronos SPIR-V/LLVM Translator. In
1056+
// some cases SPIR-V Writer emits the following sequences:
1057+
// clang-format off
1058+
//
1059+
// TypeImage 104 89 1 0 0 0 0 0 0
1060+
// FunctionParameter 104 350
1061+
// ConvertPtrToU 9 372 350
1062+
//
1063+
// clang-format on
1064+
// The last instruction is illegally trying to use an image type as a source
1065+
// in ConvertPtrToU. Unfortunately, the bug has has not been discovered
1066+
// until the switch to opaque pointers and TargetExtTy. With typed pointers,
1067+
// given that images were represented using pointers to opaque structs, the
1068+
// sequences were "legalized" in LLVM IR.
1069+
// With opaque pointers this leads to an exception in the SPIR-V Reader
1070+
// since it is illegal to use TargetExtTy as a source in LLVM's ptrtoint
1071+
// instruction. As a workaround SPIR-V Reader is emitting builtin calls to
1072+
// "__spirv_ConvertPtrToU" which can be replaced with proper ptrtoint
1073+
// instructions in LLVM IR after retyping TargetExtTy to opaque pointers.
1074+
if (CI && CI->getOperand(0)->getType()->isTargetExtTy() &&
1075+
CI->getCalledOperand() &&
1076+
CI->getCalledOperand()->getName().contains("__spirv_ConvertPtrToU"))
10501077
continue;
1078+
#endif
1079+
10511080
// Returns nullptr in case of indirect call or inline asm which was already
10521081
// considered.
10531082
auto *Child = CallEdge.second->getFunction();
10541083
if (!Child)
10551084
IGC_ASSERT_MESSAGE(
1056-
isa<CallInst>(CI) && cast<CallInst>(CI)->isIndirectCall(),
1085+
CI && CI->isIndirectCall(),
10571086
"only indirect call is exprected for a null call graph node");
10581087
if (Child && !Child->isDeclaration())
10591088
visitFunction(*Child);

IGC/VectorCompiler/lib/GenXOpts/CMTrans/GenXBTIAssignment.cpp

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -295,21 +295,93 @@ bool BTIAssignment::rewriteArguments(
295295

296296
Type *ArgTy = Arg.getType();
297297
// This code is to handle DPC++ contexts with correct OCL types.
298-
// Without actually doing something with users of args, we just
299-
// cast constant to pointer and replace arg with new value.
300-
// Later passes will do their work and clean up the mess.
298+
// We either materialize the constants in-place of args for some
299+
// instructions (known cases where we do inttoptr followed by ptrtoint) or
300+
// we just do inttoptr and replace the arg with a new value (default option,
301+
// later passes clean up the code).
301302
// FIXME(aus): proper unification of incoming IR is
302303
// required. Current approach will constantly blow all passes
303304
// where some additional case should be handled.
304-
if (ArgTy->isPointerTy())
305-
BTIConstant = IRB.CreateIntToPtr(BTIConstant, ArgTy, ".bti.cast");
306305

307-
IGC_ASSERT_MESSAGE(ArgTy == BTIConstant->getType(),
308-
"Only explicit i32 indices or opaque types are allowed "
309-
"as bti argument");
306+
// Step 1: Directly replace if Arg is an integer is of integer type already.
307+
if (ArgTy->isIntegerTy(32)) {
308+
IGC_ASSERT_MESSAGE(
309+
ArgTy == BTIConstant->getType(),
310+
"Only explicit i32 indices or opaque types are allowed "
311+
"as bti argument");
312+
Arg.replaceAllUsesWith(BTIConstant);
313+
Changed = true;
314+
continue;
315+
}
316+
317+
DenseMap<Type *, Value *> PtrConstCache;
318+
auto getPtrConst = [&](Type *PTy) -> Value * {
319+
auto It = PtrConstCache.find(PTy);
320+
if (It != PtrConstCache.end())
321+
return It->second;
322+
Value *V = IRB.CreateIntToPtr(BTIConstant, PTy, ".bti.cast");
323+
PtrConstCache[PTy] = V;
324+
return V;
325+
};
326+
327+
// Step 2: Otherwise, traverse the chain to find the end constant users.
328+
SmallVector<Instruction *, 8> WorkList;
329+
for (Use &U : llvm::make_early_inc_range(Arg.uses())) {
330+
if (auto *I = dyn_cast<Instruction>(U.getUser()))
331+
WorkList.push_back(I);
332+
}
333+
334+
while (!WorkList.empty()) {
335+
Instruction *I = WorkList.pop_back_val();
336+
337+
StoreInst *SI = dyn_cast<StoreInst>(I);
338+
if (SI && SI->getValueOperand() == &Arg) {
339+
SI->setOperand(0, BTIConstant);
340+
Changed = true;
341+
continue;
342+
}
310343

311-
Arg.replaceAllUsesWith(BTIConstant);
312-
Changed = true;
344+
#if LLVM_VERSION_MAJOR >= 16
345+
// This is a workaround due to a bug in Khronos SPIR-V/LLVM Translator. In
346+
// some cases SPIR-V Writer emits the following sequences:
347+
// clang-format off
348+
//
349+
// TypeImage 104 89 1 0 0 0 0 0 0
350+
// FunctionParameter 104 350
351+
// ConvertPtrToU 9 372 350
352+
//
353+
// clang-format on
354+
// The last instruction is illegally trying to use an image type as a
355+
// source in ConvertPtrToU. Unfortunately, the bug has has not been
356+
// discovered until the switch to opaque pointers and TargetExtTy. With
357+
// typed pointers, given that images were represented using pointers to
358+
// opaque structs, the sequences were "legalized" in LLVM IR. With opaque
359+
// pointers this leads to an exception in the SPIR-V Reader since it is
360+
// illegal to use TargetExtTy as a source in LLVM's ptrtoint instruction.
361+
// As a workaround SPIR-V Reader is emitting builtin calls to
362+
// "__spirv_ConvertPtrToU" which can be replaced with proper ptrtoint
363+
// instructions in LLVM IR after retyping TargetExtTy to opaque pointers.
364+
CallInst *CI = dyn_cast<CallInst>(I);
365+
if (CI && CI->getOperand(0)->getType()->isTargetExtTy() &&
366+
CI->getCalledOperand()->getName().contains("__spirv_ConvertPtrToU")) {
367+
IGC_ASSERT_MESSAGE(CI->getType()->isIntegerTy(32),
368+
"__spirv_ConvertPtrToU is expected to return i32!");
369+
CI->replaceAllUsesWith(BTIConstant);
370+
CI->eraseFromParent();
371+
Changed = true;
372+
continue;
373+
}
374+
#endif
375+
}
376+
377+
// Step 3: Fallback, if after targeted rewrites the argument still has uses,
378+
// provide a pointer constant. This approach does not work with opaque
379+
// pointers and TargetExtTy.
380+
if (!Arg.use_empty()) {
381+
Value *PtrConst = getPtrConst(ArgTy);
382+
Arg.replaceAllUsesWith(PtrConst);
383+
Changed = true;
384+
}
313385
}
314386

315387
return Changed;

0 commit comments

Comments
 (0)