Skip to content

Commit 505cd12

Browse files
[SPIR-V] Add validation to the test case with get_image_array_size/get_image_dim calls (#94467)
This PR is to add validation to the test case with get_image_array_size/get_image_dim calls (transcoding/check_ro_qualifier.ll). This test case didn't pass validation because of invalid emission of OpCompositeExtract instruction (Result Type must be the same type as Composite.). In order to fix the problem this PR improves type inference in general and partially addresses issues: * #91998 * #91997 A reproducer from the description of the latter issue is added as a new test case as a part of this PR.
1 parent 6b9753a commit 505cd12

9 files changed

+196
-81
lines changed

llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,11 +1459,22 @@ static bool generateImageSizeQueryInst(const SPIRV::IncomingCall *Call,
14591459
Component == 3 ? NumActualRetComponents - 1 : Component;
14601460
assert(ExtractedComposite < NumActualRetComponents &&
14611461
"Invalid composite index!");
1462+
Register TypeReg = GR->getSPIRVTypeID(Call->ReturnType);
1463+
SPIRVType *NewType = nullptr;
1464+
if (QueryResultType->getOpcode() == SPIRV::OpTypeVector) {
1465+
Register NewTypeReg = QueryResultType->getOperand(1).getReg();
1466+
if (TypeReg != NewTypeReg &&
1467+
(NewType = GR->getSPIRVTypeForVReg(NewTypeReg)) != nullptr)
1468+
TypeReg = NewTypeReg;
1469+
}
14621470
MIRBuilder.buildInstr(SPIRV::OpCompositeExtract)
14631471
.addDef(Call->ReturnRegister)
1464-
.addUse(GR->getSPIRVTypeID(Call->ReturnType))
1472+
.addUse(TypeReg)
14651473
.addUse(QueryResult)
14661474
.addImm(ExtractedComposite);
1475+
if (NewType != nullptr)
1476+
insertAssignInstr(Call->ReturnRegister, nullptr, NewType, GR, MIRBuilder,
1477+
MIRBuilder.getMF().getRegInfo());
14671478
} else {
14681479
// More than 1 component is expected, fill a new vector.
14691480
auto MIB = MIRBuilder.buildInstr(SPIRV::OpVectorShuffle)
@@ -2063,16 +2074,30 @@ static bool generateAsyncCopy(const SPIRV::IncomingCall *Call,
20632074
auto Scope = buildConstantIntReg(SPIRV::Scope::Workgroup, MIRBuilder, GR);
20642075

20652076
switch (Opcode) {
2066-
case SPIRV::OpGroupAsyncCopy:
2067-
return MIRBuilder.buildInstr(Opcode)
2068-
.addDef(Call->ReturnRegister)
2069-
.addUse(GR->getSPIRVTypeID(Call->ReturnType))
2070-
.addUse(Scope)
2071-
.addUse(Call->Arguments[0])
2072-
.addUse(Call->Arguments[1])
2073-
.addUse(Call->Arguments[2])
2074-
.addUse(buildConstantIntReg(1, MIRBuilder, GR))
2075-
.addUse(Call->Arguments[3]);
2077+
case SPIRV::OpGroupAsyncCopy: {
2078+
SPIRVType *NewType =
2079+
Call->ReturnType->getOpcode() == SPIRV::OpTypeEvent
2080+
? nullptr
2081+
: GR->getOrCreateSPIRVTypeByName("spirv.Event", MIRBuilder);
2082+
Register TypeReg = GR->getSPIRVTypeID(NewType ? NewType : Call->ReturnType);
2083+
unsigned NumArgs = Call->Arguments.size();
2084+
Register EventReg = Call->Arguments[NumArgs - 1];
2085+
bool Res = MIRBuilder.buildInstr(Opcode)
2086+
.addDef(Call->ReturnRegister)
2087+
.addUse(TypeReg)
2088+
.addUse(Scope)
2089+
.addUse(Call->Arguments[0])
2090+
.addUse(Call->Arguments[1])
2091+
.addUse(Call->Arguments[2])
2092+
.addUse(Call->Arguments.size() > 4
2093+
? Call->Arguments[3]
2094+
: buildConstantIntReg(1, MIRBuilder, GR))
2095+
.addUse(EventReg);
2096+
if (NewType != nullptr)
2097+
insertAssignInstr(Call->ReturnRegister, nullptr, NewType, GR, MIRBuilder,
2098+
MIRBuilder.getMF().getRegInfo());
2099+
return Res;
2100+
}
20762101
case SPIRV::OpGroupWaitEvents:
20772102
return MIRBuilder.buildInstr(Opcode)
20782103
.addUse(Scope)

llvm/lib/Target/SPIRV/SPIRVBuiltins.td

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,7 @@ defm : DemangledNativeBuiltin<"__spirv_SpecConstantComposite", OpenCL_std, SpecC
586586

587587
// Async Copy and Prefetch builtin records:
588588
defm : DemangledNativeBuiltin<"async_work_group_copy", OpenCL_std, AsyncCopy, 4, 4, OpGroupAsyncCopy>;
589+
defm : DemangledNativeBuiltin<"async_work_group_strided_copy", OpenCL_std, AsyncCopy, 5, 5, OpGroupAsyncCopy>;
589590
defm : DemangledNativeBuiltin<"__spirv_GroupAsyncCopy", OpenCL_std, AsyncCopy, 6, 6, OpGroupAsyncCopy>;
590591
defm : DemangledNativeBuiltin<"wait_group_events", OpenCL_std, AsyncCopy, 2, 2, OpGroupWaitEvents>;
591592
defm : DemangledNativeBuiltin<"__spirv_GroupWaitEvents", OpenCL_std, AsyncCopy, 3, 3, OpGroupWaitEvents>;

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

Lines changed: 93 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ class SPIRVEmitIntrinsics
6161
DenseMap<Instruction *, Type *> AggrConstTypes;
6262
DenseSet<Instruction *> AggrStores;
6363

64-
// a registry of created Intrinsic::spv_assign_ptr_type instructions
65-
DenseMap<Value *, CallInst *> AssignPtrTypeInstr;
66-
6764
// deduce element type of untyped pointers
6865
Type *deduceElementType(Value *I);
6966
Type *deduceElementTypeHelper(Value *I);
@@ -98,14 +95,16 @@ class SPIRVEmitIntrinsics
9895
return B.CreateIntrinsic(IntrID, {Types}, Args);
9996
}
10097

98+
void buildAssignType(IRBuilder<> &B, Type *ElemTy, Value *Arg);
10199
void buildAssignPtr(IRBuilder<> &B, Type *ElemTy, Value *Arg);
100+
void updateAssignType(CallInst *AssignCI, Value *Arg, Value *OfType);
102101

103102
void replaceMemInstrUses(Instruction *Old, Instruction *New, IRBuilder<> &B);
104103
void processInstrAfterVisit(Instruction *I, IRBuilder<> &B);
105104
void insertAssignPtrTypeIntrs(Instruction *I, IRBuilder<> &B);
106105
void insertAssignTypeIntrs(Instruction *I, IRBuilder<> &B);
107-
void insertAssignTypeInstrForTargetExtTypes(TargetExtType *AssignedType,
108-
Value *V, IRBuilder<> &B);
106+
void insertAssignPtrTypeTargetExt(TargetExtType *AssignedType, Value *V,
107+
IRBuilder<> &B);
109108
void replacePointerOperandWithPtrCast(Instruction *I, Value *Pointer,
110109
Type *ExpectedElementType,
111110
unsigned OperandToReplace,
@@ -218,15 +217,39 @@ static inline void reportFatalOnTokenType(const Instruction *I) {
218217
false);
219218
}
220219

220+
void SPIRVEmitIntrinsics::buildAssignType(IRBuilder<> &B, Type *Ty,
221+
Value *Arg) {
222+
Value *OfType = PoisonValue::get(Ty);
223+
CallInst *AssignCI = buildIntrWithMD(Intrinsic::spv_assign_type,
224+
{Arg->getType()}, OfType, Arg, {}, B);
225+
GR->addAssignPtrTypeInstr(Arg, AssignCI);
226+
}
227+
221228
void SPIRVEmitIntrinsics::buildAssignPtr(IRBuilder<> &B, Type *ElemTy,
222229
Value *Arg) {
223-
CallInst *AssignPtrTyCI =
224-
buildIntrWithMD(Intrinsic::spv_assign_ptr_type, {Arg->getType()},
225-
Constant::getNullValue(ElemTy), Arg,
226-
{B.getInt32(getPointerAddressSpace(Arg->getType()))}, B);
230+
Value *OfType = PoisonValue::get(ElemTy);
231+
CallInst *AssignPtrTyCI = buildIntrWithMD(
232+
Intrinsic::spv_assign_ptr_type, {Arg->getType()}, OfType, Arg,
233+
{B.getInt32(getPointerAddressSpace(Arg->getType()))}, B);
227234
GR->addDeducedElementType(AssignPtrTyCI, ElemTy);
228235
GR->addDeducedElementType(Arg, ElemTy);
229-
AssignPtrTypeInstr[Arg] = AssignPtrTyCI;
236+
GR->addAssignPtrTypeInstr(Arg, AssignPtrTyCI);
237+
}
238+
239+
void SPIRVEmitIntrinsics::updateAssignType(CallInst *AssignCI, Value *Arg,
240+
Value *OfType) {
241+
LLVMContext &Ctx = Arg->getContext();
242+
AssignCI->setArgOperand(
243+
1, MetadataAsValue::get(
244+
Ctx, MDNode::get(Ctx, ValueAsMetadata::getConstant(OfType))));
245+
if (cast<IntrinsicInst>(AssignCI)->getIntrinsicID() !=
246+
Intrinsic::spv_assign_ptr_type)
247+
return;
248+
249+
// update association with the pointee type
250+
Type *ElemTy = OfType->getType();
251+
GR->addDeducedElementType(AssignCI, ElemTy);
252+
GR->addDeducedElementType(Arg, ElemTy);
230253
}
231254

232255
// Set element pointer type to the given value of ValueTy and tries to
@@ -513,19 +536,16 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(Instruction *I) {
513536
if (!Ty) {
514537
GR->addDeducedElementType(Op, KnownElemTy);
515538
// check if there is existing Intrinsic::spv_assign_ptr_type instruction
516-
auto It = AssignPtrTypeInstr.find(Op);
517-
if (It == AssignPtrTypeInstr.end()) {
539+
CallInst *AssignCI = GR->findAssignPtrTypeInstr(Op);
540+
if (AssignCI == nullptr) {
518541
Instruction *User = dyn_cast<Instruction>(Op->use_begin()->get());
519542
setInsertPointSkippingPhis(B, User ? User->getNextNode() : I);
520543
CallInst *CI =
521544
buildIntrWithMD(Intrinsic::spv_assign_ptr_type, {OpTy}, OpTyVal, Op,
522545
{B.getInt32(getPointerAddressSpace(OpTy))}, B);
523-
AssignPtrTypeInstr[Op] = CI;
546+
GR->addAssignPtrTypeInstr(Op, CI);
524547
} else {
525-
It->second->setArgOperand(
526-
1,
527-
MetadataAsValue::get(
528-
Ctx, MDNode::get(Ctx, ValueAsMetadata::getConstant(OpTyVal))));
548+
updateAssignType(AssignCI, Op, OpTyVal);
529549
}
530550
} else {
531551
if (auto *OpI = dyn_cast<Instruction>(Op)) {
@@ -559,7 +579,9 @@ void SPIRVEmitIntrinsics::replaceMemInstrUses(Instruction *Old,
559579
if (isAssignTypeInstr(U)) {
560580
B.SetInsertPoint(U);
561581
SmallVector<Value *, 2> Args = {New, U->getOperand(1)};
562-
B.CreateIntrinsic(Intrinsic::spv_assign_type, {New->getType()}, Args);
582+
CallInst *AssignCI =
583+
B.CreateIntrinsic(Intrinsic::spv_assign_type, {New->getType()}, Args);
584+
GR->addAssignPtrTypeInstr(New, AssignCI);
563585
U->eraseFromParent();
564586
} else if (isMemInstrToReplace(U) || isa<ReturnInst>(U) ||
565587
isa<CallInst>(U)) {
@@ -751,33 +773,39 @@ Instruction *SPIRVEmitIntrinsics::visitBitCastInst(BitCastInst &I) {
751773
return NewI;
752774
}
753775

754-
void SPIRVEmitIntrinsics::insertAssignTypeInstrForTargetExtTypes(
776+
void SPIRVEmitIntrinsics::insertAssignPtrTypeTargetExt(
755777
TargetExtType *AssignedType, Value *V, IRBuilder<> &B) {
756-
// Do not emit spv_assign_type if the V is of the AssignedType already.
757-
if (V->getType() == AssignedType)
758-
return;
778+
Type *VTy = V->getType();
759779

760-
// Do not emit spv_assign_type if there is one already targetting V. If the
761-
// found spv_assign_type assigns a type different than AssignedType, report an
762-
// error. Builtin types cannot be redeclared or casted.
763-
for (auto User : V->users()) {
764-
auto *II = dyn_cast<IntrinsicInst>(User);
765-
if (!II || II->getIntrinsicID() != Intrinsic::spv_assign_type)
766-
continue;
780+
// A couple of sanity checks.
781+
assert(isPointerTy(VTy) && "Expect a pointer type!");
782+
if (auto PType = dyn_cast<TypedPointerType>(VTy))
783+
if (PType->getElementType() != AssignedType)
784+
report_fatal_error("Unexpected pointer element type!");
767785

768-
MetadataAsValue *VMD = cast<MetadataAsValue>(II->getOperand(1));
769-
Type *BuiltinType =
770-
dyn_cast<ConstantAsMetadata>(VMD->getMetadata())->getType();
771-
if (BuiltinType != AssignedType)
772-
report_fatal_error("Type mismatch " + BuiltinType->getTargetExtName() +
773-
"/" + AssignedType->getTargetExtName() +
774-
" for value " + V->getName(),
775-
false);
786+
CallInst *AssignCI = GR->findAssignPtrTypeInstr(V);
787+
if (!AssignCI) {
788+
buildAssignType(B, AssignedType, V);
776789
return;
777790
}
778791

779-
Constant *Const = UndefValue::get(AssignedType);
780-
buildIntrWithMD(Intrinsic::spv_assign_type, {V->getType()}, Const, V, {}, B);
792+
Type *CurrentType =
793+
dyn_cast<ConstantAsMetadata>(
794+
cast<MetadataAsValue>(AssignCI->getOperand(1))->getMetadata())
795+
->getType();
796+
if (CurrentType == AssignedType)
797+
return;
798+
799+
// Builtin types cannot be redeclared or casted.
800+
if (CurrentType->isTargetExtTy())
801+
report_fatal_error("Type mismatch " + CurrentType->getTargetExtName() +
802+
"/" + AssignedType->getTargetExtName() +
803+
" for value " + V->getName(),
804+
false);
805+
806+
// Our previous guess about the type seems to be wrong, let's update
807+
// inferred type according to a new, more precise type information.
808+
updateAssignType(AssignCI, V, PoisonValue::get(AssignedType));
781809
}
782810

783811
void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
@@ -850,7 +878,7 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
850878
ExpectedElementTypeConst, Pointer, {B.getInt32(AddressSpace)}, B);
851879
GR->addDeducedElementType(CI, ExpectedElementType);
852880
GR->addDeducedElementType(Pointer, ExpectedElementType);
853-
AssignPtrTypeInstr[Pointer] = CI;
881+
GR->addAssignPtrTypeInstr(Pointer, CI);
854882
return;
855883
}
856884

@@ -929,8 +957,7 @@ void SPIRVEmitIntrinsics::insertPtrCastOrAssignTypeInstr(Instruction *I,
929957

930958
for (unsigned OpIdx = 0; OpIdx < CI->arg_size(); OpIdx++) {
931959
Value *ArgOperand = CI->getArgOperand(OpIdx);
932-
if (!isa<PointerType>(ArgOperand->getType()) &&
933-
!isa<TypedPointerType>(ArgOperand->getType()))
960+
if (!isPointerTy(ArgOperand->getType()))
934961
continue;
935962

936963
// Constants (nulls/undefs) are handled in insertAssignPtrTypeIntrs()
@@ -952,8 +979,8 @@ void SPIRVEmitIntrinsics::insertPtrCastOrAssignTypeInstr(Instruction *I,
952979
continue;
953980

954981
if (ExpectedType->isTargetExtTy())
955-
insertAssignTypeInstrForTargetExtTypes(cast<TargetExtType>(ExpectedType),
956-
ArgOperand, B);
982+
insertAssignPtrTypeTargetExt(cast<TargetExtType>(ExpectedType),
983+
ArgOperand, B);
957984
else
958985
replacePointerOperandWithPtrCast(CI, ArgOperand, ExpectedType, OpIdx, B);
959986
}
@@ -1145,7 +1172,7 @@ void SPIRVEmitIntrinsics::insertAssignPtrTypeIntrs(Instruction *I,
11451172
CallInst *CI = buildIntrWithMD(Intrinsic::spv_assign_ptr_type, {I->getType()},
11461173
EltTyConst, I, {B.getInt32(AddressSpace)}, B);
11471174
GR->addDeducedElementType(CI, ElemTy);
1148-
AssignPtrTypeInstr[I] = CI;
1175+
GR->addAssignPtrTypeInstr(I, CI);
11491176
}
11501177

11511178
void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I,
@@ -1164,20 +1191,32 @@ void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I,
11641191
TypeToAssign = It->second;
11651192
}
11661193
}
1167-
Constant *Const = UndefValue::get(TypeToAssign);
1168-
buildIntrWithMD(Intrinsic::spv_assign_type, {Ty}, Const, I, {}, B);
1194+
buildAssignType(B, TypeToAssign, I);
11691195
}
11701196
for (const auto &Op : I->operands()) {
11711197
if (isa<ConstantPointerNull>(Op) || isa<UndefValue>(Op) ||
11721198
// Check GetElementPtrConstantExpr case.
11731199
(isa<ConstantExpr>(Op) && isa<GEPOperator>(Op))) {
11741200
setInsertPointSkippingPhis(B, I);
1175-
if (isa<UndefValue>(Op) && Op->getType()->isAggregateType())
1176-
buildIntrWithMD(Intrinsic::spv_assign_type, {B.getInt32Ty()}, Op,
1177-
UndefValue::get(B.getInt32Ty()), {}, B);
1178-
else if (!isa<Instruction>(Op))
1179-
buildIntrWithMD(Intrinsic::spv_assign_type, {Op->getType()}, Op, Op, {},
1180-
B);
1201+
Type *OpTy = Op->getType();
1202+
if (isa<UndefValue>(Op) && OpTy->isAggregateType()) {
1203+
CallInst *AssignCI =
1204+
buildIntrWithMD(Intrinsic::spv_assign_type, {B.getInt32Ty()}, Op,
1205+
UndefValue::get(B.getInt32Ty()), {}, B);
1206+
GR->addAssignPtrTypeInstr(Op, AssignCI);
1207+
} else if (!isa<Instruction>(Op)) {
1208+
Type *OpTy = Op->getType();
1209+
if (auto PType = dyn_cast<TypedPointerType>(OpTy)) {
1210+
buildAssignPtr(B, PType->getElementType(), Op);
1211+
} else if (isPointerTy(OpTy)) {
1212+
Type *ElemTy = GR->findDeducedElementType(Op);
1213+
buildAssignPtr(B, ElemTy ? ElemTy : deduceElementType(Op), Op);
1214+
} else {
1215+
CallInst *AssignCI = buildIntrWithMD(Intrinsic::spv_assign_type,
1216+
{OpTy}, Op, Op, {}, B);
1217+
GR->addAssignPtrTypeInstr(Op, AssignCI);
1218+
}
1219+
}
11811220
}
11821221
}
11831222
}
@@ -1368,14 +1407,12 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
13681407
continue;
13691408

13701409
insertAssignPtrTypeIntrs(I, B);
1410+
deduceOperandElementType(I);
13711411
insertAssignTypeIntrs(I, B);
13721412
insertPtrCastOrAssignTypeInstr(I, B);
13731413
insertSpirvDecorations(I, B);
13741414
}
13751415

1376-
for (auto &I : instructions(Func))
1377-
deduceOperandElementType(&I);
1378-
13791416
for (auto *I : Worklist) {
13801417
TrackConstants = true;
13811418
if (!I->getType()->isVoidTy() || isa<StoreInst>(I))

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,11 @@ class SPIRVGlobalRegistry {
7373
// untyped pointers.
7474
DenseMap<Value *, Type *> DeducedElTys;
7575
// Maps composite values to deduced types where untyped pointers are replaced
76-
// with typed ones
76+
// with typed ones.
7777
DenseMap<Value *, Type *> DeducedNestedTys;
78+
// Maps values to "assign type" calls, thus being a registry of created
79+
// Intrinsic::spv_assign_ptr_type instructions.
80+
DenseMap<Value *, CallInst *> AssignPtrTypeInstr;
7881

7982
// Add a new OpTypeXXX instruction without checking for duplicates.
8083
SPIRVType *createSPIRVType(const Type *Type, MachineIRBuilder &MIRBuilder,
@@ -149,6 +152,17 @@ class SPIRVGlobalRegistry {
149152
return It == FunResPointerTypes.end() ? nullptr : It->second;
150153
}
151154

155+
// A registry of "assign type" records:
156+
// - Add a record.
157+
void addAssignPtrTypeInstr(Value *Val, CallInst *AssignPtrTyCI) {
158+
AssignPtrTypeInstr[Val] = AssignPtrTyCI;
159+
}
160+
// - Find a record.
161+
CallInst *findAssignPtrTypeInstr(const Value *Val) {
162+
auto It = AssignPtrTypeInstr.find(Val);
163+
return It == AssignPtrTypeInstr.end() ? nullptr : It->second;
164+
}
165+
152166
// Deduced element types of untyped pointers and composites:
153167
// - Add a record to the map of deduced element types.
154168
void addDeducedElementType(Value *Val, Type *Ty) { DeducedElTys[Val] = Ty; }

llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,8 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
417417
MachineInstr *Def = MRI.getVRegDef(Reg);
418418
assert(Def && "Expecting an instruction that defines the register");
419419
// G_GLOBAL_VALUE already has type info.
420-
if (Def->getOpcode() != TargetOpcode::G_GLOBAL_VALUE)
420+
if (Def->getOpcode() != TargetOpcode::G_GLOBAL_VALUE &&
421+
Def->getOpcode() != SPIRV::ASSIGN_TYPE)
421422
insertAssignInstr(Reg, nullptr, AssignedPtrType, GR, MIB,
422423
MF.getRegInfo());
423424
ToErase.push_back(&MI);
@@ -427,7 +428,8 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
427428
MachineInstr *Def = MRI.getVRegDef(Reg);
428429
assert(Def && "Expecting an instruction that defines the register");
429430
// G_GLOBAL_VALUE already has type info.
430-
if (Def->getOpcode() != TargetOpcode::G_GLOBAL_VALUE)
431+
if (Def->getOpcode() != TargetOpcode::G_GLOBAL_VALUE &&
432+
Def->getOpcode() != SPIRV::ASSIGN_TYPE)
431433
insertAssignInstr(Reg, Ty, nullptr, GR, MIB, MF.getRegInfo());
432434
ToErase.push_back(&MI);
433435
} else if (MIOp == TargetOpcode::G_CONSTANT ||

0 commit comments

Comments
 (0)