Skip to content

Commit

Permalink
Revert "SPV_KHR_untyped_pointers - implement OpTypeUntypedPointerKHR (#…
Browse files Browse the repository at this point in the history
…2687)"

This reverts commit ab78ffb.
  • Loading branch information
jsji committed Sep 22, 2024
1 parent dadd5b3 commit b1f2327
Show file tree
Hide file tree
Showing 19 changed files with 55 additions and 372 deletions.
1 change: 0 additions & 1 deletion llvm-spirv/include/LLVMSPIRVExtensions.inc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ EXT(SPV_KHR_subgroup_rotate)
EXT(SPV_KHR_non_semantic_info)
EXT(SPV_KHR_shader_clock)
EXT(SPV_KHR_cooperative_matrix)
EXT(SPV_KHR_untyped_pointers)
EXT(SPV_INTEL_subgroups)
EXT(SPV_INTEL_media_block_io)
EXT(SPV_INTEL_device_side_avc_motion_estimation)
Expand Down
7 changes: 0 additions & 7 deletions llvm-spirv/lib/SPIRV/SPIRVReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,11 +357,6 @@ Type *SPIRVToLLVM::transType(SPIRVType *T, bool UseTPT) {
return TypedPointerType::get(ElementTy, AS);
return mapType(T, PointerType::get(ElementTy, AS));
}
case OpTypeUntypedPointerKHR: {
const unsigned AS =
SPIRSPIRVAddrSpaceMap::rmap(T->getPointerStorageClass());
return mapType(T, PointerType::get(*Context, AS));
}
case OpTypeVector:
return mapType(T,
FixedVectorType::get(transType(T->getVectorComponentType()),
Expand Down Expand Up @@ -565,8 +560,6 @@ std::string SPIRVToLLVM::transTypeToOCLTypeName(SPIRVType *T, bool IsSigned) {
}
return transTypeToOCLTypeName(ET) + "*";
}
case OpTypeUntypedPointerKHR:
return "int*";
case OpTypeVector:
return transTypeToOCLTypeName(T->getVectorComponentType()) +
T->getVectorComponentCount();
Expand Down
56 changes: 14 additions & 42 deletions llvm-spirv/lib/SPIRV/SPIRVWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ SPIRVType *LLVMToSPIRVBase::transType(Type *T) {
// A pointer to image or pipe type in LLVM is translated to a SPIRV
// (non-pointer) image or pipe type.
if (T->isPointerTy()) {
auto AddrSpc = T->getPointerAddressSpace();
auto *ET = Type::getInt8Ty(T->getContext());
auto AddrSpc = T->getPointerAddressSpace();
return transPointerType(ET, AddrSpc);
}

Expand Down Expand Up @@ -720,6 +720,7 @@ SPIRVType *LLVMToSPIRVBase::transPointerType(Type *ET, unsigned AddrSpc) {
transType(ET)));
}
} else {
SPIRVType *ElementType = transType(ET);
// ET, as a recursive type, may contain exactly the same pointer T, so it
// may happen that after translation of ET we already have translated T,
// added the translated pointer to the SPIR-V module and mapped T to this
Expand All @@ -728,17 +729,7 @@ SPIRVType *LLVMToSPIRVBase::transPointerType(Type *ET, unsigned AddrSpc) {
if (Loc != PointeeTypeMap.end()) {
return Loc->second;
}

SPIRVType *ElementType = nullptr;
SPIRVType *TranslatedTy = nullptr;
if (ET->isPointerTy() &&
BM->isAllowedToUseExtension(ExtensionID::SPV_KHR_untyped_pointers)) {
TranslatedTy = BM->addUntypedPointerKHRType(
SPIRSPIRVAddrSpaceMap::map(static_cast<SPIRAddressSpace>(AddrSpc)));
} else {
ElementType = transType(ET);
TranslatedTy = transPointerType(ElementType, AddrSpc);
}
SPIRVType *TranslatedTy = transPointerType(ElementType, AddrSpc);
PointeeTypeMap[TypeKey] = TranslatedTy;
return TranslatedTy;
}
Expand All @@ -753,16 +744,8 @@ SPIRVType *LLVMToSPIRVBase::transPointerType(SPIRVType *ET, unsigned AddrSpc) {
if (Loc != PointeeTypeMap.end())
return Loc->second;

SPIRVType *TranslatedTy = nullptr;
if (BM->isAllowedToUseExtension(ExtensionID::SPV_KHR_untyped_pointers) &&
!(ET->isTypeArray() || ET->isTypeVector() || ET->isTypeImage() ||
ET->isTypeSampler() || ET->isTypePipe())) {
TranslatedTy = BM->addUntypedPointerKHRType(
SPIRSPIRVAddrSpaceMap::map(static_cast<SPIRAddressSpace>(AddrSpc)));
} else {
TranslatedTy = BM->addPointerType(
SPIRSPIRVAddrSpaceMap::map(static_cast<SPIRAddressSpace>(AddrSpc)), ET);
}
SPIRVType *TranslatedTy = BM->addPointerType(
SPIRSPIRVAddrSpaceMap::map(static_cast<SPIRAddressSpace>(AddrSpc)), ET);
PointeeTypeMap[TypeKey] = TranslatedTy;
return TranslatedTy;
}
Expand Down Expand Up @@ -2213,13 +2196,8 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
MemoryAccessNoAliasINTELMaskMask);
if (MemoryAccess.front() == 0)
MemoryAccess.clear();
return mapValue(
V,
BM->addLoadInst(
transValue(LD->getPointerOperand(), BB), MemoryAccess, BB,
BM->isAllowedToUseExtension(ExtensionID::SPV_KHR_untyped_pointers)
? transType(LD->getType())
: nullptr));
return mapValue(V, BM->addLoadInst(transValue(LD->getPointerOperand(), BB),
MemoryAccess, BB));
}

if (BinaryOperator *B = dyn_cast<BinaryOperator>(V)) {
Expand Down Expand Up @@ -2429,17 +2407,14 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,

if (auto *Phi = dyn_cast<PHINode>(V)) {
std::vector<SPIRVValue *> IncomingPairs;
SPIRVType *Ty = transScavengedType(Phi);

for (size_t I = 0, E = Phi->getNumIncomingValues(); I != E; ++I) {
SPIRVValue *Val = transValue(Phi->getIncomingValue(I), BB, true,
FuncTransMode::Pointer);
if (Val->getType() != Ty)
Val = BM->addUnaryInst(OpBitcast, Ty, Val, BB);
IncomingPairs.push_back(Val);
IncomingPairs.push_back(transValue(Phi->getIncomingValue(I), BB, true,
FuncTransMode::Pointer));
IncomingPairs.push_back(transValue(Phi->getIncomingBlock(I), nullptr));
}
return mapValue(V, BM->addPhiInst(Ty, IncomingPairs, BB));
return mapValue(V,
BM->addPhiInst(transScavengedType(Phi), IncomingPairs, BB));
}

if (auto *Ext = dyn_cast<ExtractValueInst>(V)) {
Expand Down Expand Up @@ -6696,12 +6671,9 @@ LLVMToSPIRVBase::transBuiltinToInstWithoutDecoration(Op OC, CallInst *CI,
assert((Pointee == Args[I] || !isa<Function>(Pointee)) &&
"Illegal use of a function pointer type");
}
if (!SPI->isOperandLiteral(I)) {
SPIRVValue *Val = transValue(Args[I], BB);
SPArgs.push_back(Val->getId());
} else {
SPArgs.push_back(cast<ConstantInt>(Args[I])->getZExtValue());
}
SPArgs.push_back(SPI->isOperandLiteral(I)
? cast<ConstantInt>(Args[I])->getZExtValue()
: transValue(Args[I], BB)->getId());
}
BM->addInstTemplate(SPI, SPArgs, BB, SPRetTy);
if (!SPRetTy || !SPRetTy->isTypeStruct())
Expand Down
34 changes: 9 additions & 25 deletions llvm-spirv/lib/SPIRV/libSPIRV/SPIRVInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -568,15 +568,9 @@ class SPIRVStore : public SPIRVInstruction, public SPIRVMemoryAccess {
SPIRVInstruction::validate();
if (getSrc()->isForward() || getDst()->isForward())
return;
assert(
(getValueType(PtrId)
->getPointerElementType()
->isTypeUntypedPointerKHR() ||
// TODO: This check should be removed once we support untyped
// variables.
getValueType(ValId)->isTypeUntypedPointerKHR() ||
getValueType(PtrId)->getPointerElementType() == getValueType(ValId)) &&
"Inconsistent operand types");
assert(getValueType(PtrId)->getPointerElementType() ==
getValueType(ValId) &&
"Inconsistent operand types");
}

private:
Expand All @@ -591,12 +585,11 @@ class SPIRVLoad : public SPIRVInstruction, public SPIRVMemoryAccess {
// Complete constructor
SPIRVLoad(SPIRVId TheId, SPIRVId PointerId,
const std::vector<SPIRVWord> &TheMemoryAccess,
SPIRVBasicBlock *TheBB, SPIRVType *TheType = nullptr)
SPIRVBasicBlock *TheBB)
: SPIRVInstruction(
FixedWords + TheMemoryAccess.size(), OpLoad,
TheType ? TheType
: TheBB->getValueType(PointerId)->getPointerElementType(),
TheId, TheBB),
TheBB->getValueType(PointerId)->getPointerElementType(), TheId,
TheBB),
SPIRVMemoryAccess(TheMemoryAccess), PtrId(PointerId),
MemoryAccess(TheMemoryAccess) {
validate();
Expand Down Expand Up @@ -626,12 +619,6 @@ class SPIRVLoad : public SPIRVInstruction, public SPIRVMemoryAccess {
void validate() const override {
SPIRVInstruction::validate();
assert((getValue(PtrId)->isForward() ||
getValueType(PtrId)
->getPointerElementType()
->isTypeUntypedPointerKHR() ||
// TODO: This check should be removed once we support untyped
// variables.
Type->isTypeUntypedPointerKHR() ||
Type == getValueType(PtrId)->getPointerElementType()) &&
"Inconsistent types");
}
Expand Down Expand Up @@ -2014,8 +2001,7 @@ class SPIRVCompositeExtractBase : public SPIRVInstTemplateBase {
(void)Composite;
assert(getValueType(Composite)->isTypeArray() ||
getValueType(Composite)->isTypeStruct() ||
getValueType(Composite)->isTypeVector() ||
getValueType(Composite)->isTypeUntypedPointerKHR());
getValueType(Composite)->isTypeVector());
}
};

Expand All @@ -2041,8 +2027,7 @@ class SPIRVCompositeInsertBase : public SPIRVInstTemplateBase {
(void)Composite;
assert(getValueType(Composite)->isTypeArray() ||
getValueType(Composite)->isTypeStruct() ||
getValueType(Composite)->isTypeVector() ||
getValueType(Composite)->isTypeUntypedPointerKHR());
getValueType(Composite)->isTypeVector());
assert(Type == getValueType(Composite));
}
};
Expand Down Expand Up @@ -2389,8 +2374,7 @@ template <Op OC> class SPIRVLifetime : public SPIRVInstruction {
// Signedness of 1, its sign bit cannot be set.
if (!(ObjType->getPointerElementType()->isTypeVoid() ||
// (void *) is i8* in LLVM IR
ObjType->getPointerElementType()->isTypeInt(8) ||
ObjType->getPointerElementType()->isTypeUntypedPointerKHR()) ||
ObjType->getPointerElementType()->isTypeInt(8)) ||
!Module->hasCapability(CapabilityAddresses))
assert(Size == 0 && "Size must be 0");
}
Expand Down
28 changes: 5 additions & 23 deletions llvm-spirv/lib/SPIRV/libSPIRV/SPIRVModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,6 @@ class SPIRVModuleImpl : public SPIRVModule {
SPIRVTypeInt *addIntegerType(unsigned BitWidth) override;
SPIRVTypeOpaque *addOpaqueType(const std::string &) override;
SPIRVTypePointer *addPointerType(SPIRVStorageClassKind, SPIRVType *) override;
SPIRVTypeUntypedPointerKHR *
addUntypedPointerKHRType(SPIRVStorageClassKind) override;
SPIRVTypeImage *addImageType(SPIRVType *,
const SPIRVTypeImageDescriptor &) override;
SPIRVTypeImage *addImageType(SPIRVType *, const SPIRVTypeImageDescriptor &,
Expand Down Expand Up @@ -355,7 +353,7 @@ class SPIRVModuleImpl : public SPIRVModule {
SPIRVInstruction *addCmpInst(Op, SPIRVType *, SPIRVValue *, SPIRVValue *,
SPIRVBasicBlock *) override;
SPIRVInstruction *addLoadInst(SPIRVValue *, const std::vector<SPIRVWord> &,
SPIRVBasicBlock *, SPIRVType *) override;
SPIRVBasicBlock *) override;
SPIRVInstruction *addPhiInst(SPIRVType *, std::vector<SPIRVValue *>,
SPIRVBasicBlock *) override;
SPIRVInstruction *addCompositeConstructInst(SPIRVType *,
Expand Down Expand Up @@ -565,8 +563,6 @@ class SPIRVModuleImpl : public SPIRVModule {
SPIRVUnknownStructFieldMap UnknownStructFieldMap;
SPIRVTypeBool *BoolTy;
SPIRVTypeVoid *VoidTy;
SmallDenseMap<SPIRVStorageClassKind, SPIRVTypeUntypedPointerKHR *>
UntypedPtrTyMap;
SmallDenseMap<unsigned, SPIRVTypeInt *, 4> IntTypeMap;
SmallDenseMap<unsigned, SPIRVTypeFloat *, 4> FloatTypeMap;
SmallDenseMap<std::pair<unsigned, SPIRVType *>, SPIRVTypePointer *, 4>
Expand Down Expand Up @@ -1018,17 +1014,6 @@ SPIRVModuleImpl::addPointerType(SPIRVStorageClassKind StorageClass,
return addType(Ty);
}

SPIRVTypeUntypedPointerKHR *
SPIRVModuleImpl::addUntypedPointerKHRType(SPIRVStorageClassKind StorageClass) {
auto Loc = UntypedPtrTyMap.find(StorageClass);
if (Loc != UntypedPtrTyMap.end())
return Loc->second;

auto *Ty = new SPIRVTypeUntypedPointerKHR(this, getId(), StorageClass);
UntypedPtrTyMap[StorageClass] = Ty;
return addType(Ty);
}

SPIRVTypeFunction *SPIRVModuleImpl::addFunctionType(
SPIRVType *ReturnType, const std::vector<SPIRVType *> &ParameterTypes) {
return addType(
Expand Down Expand Up @@ -1445,10 +1430,9 @@ SPIRVModuleImpl::addInstruction(SPIRVInstruction *Inst, SPIRVBasicBlock *BB,
SPIRVInstruction *
SPIRVModuleImpl::addLoadInst(SPIRVValue *Source,
const std::vector<SPIRVWord> &TheMemoryAccess,
SPIRVBasicBlock *BB, SPIRVType *TheType) {
SPIRVBasicBlock *BB) {
return addInstruction(
new SPIRVLoad(getId(), Source->getId(), TheMemoryAccess, BB, TheType),
BB);
new SPIRVLoad(getId(), Source->getId(), TheMemoryAccess, BB), BB);
}

SPIRVInstruction *
Expand Down Expand Up @@ -1941,13 +1925,11 @@ class TopologicalSort {
// We've found a recursive data type, e.g. a structure having a member
// which is a pointer to the same structure.
State = Unvisited; // Forget about it
if (E->getOpCode() == OpTypePointer ||
E->getOpCode() == OpTypeUntypedPointerKHR) {
if (E->getOpCode() == OpTypePointer) {
// If we have a pointer in the recursive chain, we can break the
// cyclic dependency by inserting a forward declaration of that
// pointer.
SPIRVTypePointerBase<> *Ptr =
static_cast<SPIRVTypePointerBase<> *>(E);
SPIRVTypePointer *Ptr = static_cast<SPIRVTypePointer *>(E);
SPIRVModule *BM = E->getModule();
ForwardPointerSet.insert(BM->add(new SPIRVTypeForwardPointer(
BM, Ptr->getId(), Ptr->getPointerStorageClass())));
Expand Down
6 changes: 1 addition & 5 deletions llvm-spirv/lib/SPIRV/libSPIRV/SPIRVModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ class SPIRVTypeFunction;
class SPIRVTypeInt;
class SPIRVTypeOpaque;
class SPIRVTypePointer;
class SPIRVTypeUntypedPointerKHR;
class SPIRVTypeImage;
class SPIRVTypeSampler;
class SPIRVTypeSampledImage;
Expand Down Expand Up @@ -258,8 +257,6 @@ class SPIRVModule {
virtual SPIRVTypeOpaque *addOpaqueType(const std::string &) = 0;
virtual SPIRVTypePointer *addPointerType(SPIRVStorageClassKind,
SPIRVType *) = 0;
virtual SPIRVTypeUntypedPointerKHR *
addUntypedPointerKHRType(SPIRVStorageClassKind) = 0;
virtual SPIRVTypeStruct *openStructType(unsigned, const std::string &) = 0;
virtual SPIRVEntry *addTypeStructContinuedINTEL(unsigned NumMembers) = 0;
virtual void closeStructType(SPIRVTypeStruct *, bool) = 0;
Expand Down Expand Up @@ -399,8 +396,7 @@ class SPIRVModule {
SPIRVBasicBlock *BB, SPIRVType *Ty) = 0;
virtual SPIRVInstruction *addLoadInst(SPIRVValue *,
const std::vector<SPIRVWord> &,
SPIRVBasicBlock *,
SPIRVType *TheType = nullptr) = 0;
SPIRVBasicBlock *) = 0;
virtual SPIRVInstruction *addLifetimeInst(Op OC, SPIRVValue *Object,
SPIRVWord Size,
SPIRVBasicBlock *BB) = 0;
Expand Down
1 change: 0 additions & 1 deletion llvm-spirv/lib/SPIRV/libSPIRV/SPIRVNameMapEnum.h
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,6 @@ template <> inline void SPIRVMap<Capability, std::string>::init() {
add(CapabilityRoundingModeRTZ, "RoundingModeRTZ");
add(CapabilityRayQueryProvisionalKHR, "RayQueryProvisionalKHR");
add(CapabilityRayQueryKHR, "RayQueryKHR");
add(CapabilityUntypedPointersKHR, "UntypedPointersKHR");
add(CapabilityRayTraversalPrimitiveCullingKHR,
"RayTraversalPrimitiveCullingKHR");
add(CapabilityRayTracingKHR, "RayTracingKHR");
Expand Down
3 changes: 1 addition & 2 deletions llvm-spirv/lib/SPIRV/libSPIRV/SPIRVOpCode.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,7 @@ inline bool isTypeOpCode(Op OpCode) {
OC == internal::OpTypeJointMatrixINTEL ||
OC == internal::OpTypeJointMatrixINTELv2 ||
OC == OpTypeCooperativeMatrixKHR ||
OC == internal::OpTypeTaskSequenceINTEL ||
OC == OpTypeUntypedPointerKHR;
OC == internal::OpTypeTaskSequenceINTEL;
}

inline bool isSpecConstantOpCode(Op OpCode) {
Expand Down
4 changes: 3 additions & 1 deletion llvm-spirv/lib/SPIRV/libSPIRV/SPIRVOpCodeEnum.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,11 +329,13 @@ _SPIRV_OP(GroupNonUniformBitwiseXor, 361)
_SPIRV_OP(GroupNonUniformLogicalAnd, 362)
_SPIRV_OP(GroupNonUniformLogicalOr, 363)
_SPIRV_OP(GroupNonUniformLogicalXor, 364)
_SPIRV_OP(PtrEqual, 401)
_SPIRV_OP(PtrNotEqual, 402)
_SPIRV_OP(PtrDiff, 403)
_SPIRV_OP(CopyLogical, 400)
_SPIRV_OP(PtrEqual, 401)
_SPIRV_OP(PtrNotEqual, 402)
_SPIRV_OP(PtrDiff, 403)
_SPIRV_OP(TypeUntypedPointerKHR, 4417)
_SPIRV_OP(GroupNonUniformRotateKHR, 4431)
_SPIRV_OP(SDotKHR, 4450)
_SPIRV_OP(UDotKHR, 4451)
Expand Down
16 changes: 3 additions & 13 deletions llvm-spirv/lib/SPIRV/libSPIRV/SPIRVType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,12 @@ SPIRVType *SPIRVType::getFunctionReturnType() const {
}

SPIRVType *SPIRVType::getPointerElementType() const {
assert((OpCode == OpTypePointer || OpCode == OpTypeUntypedPointerKHR) &&
"Not a pointer type");
if (OpCode == OpTypeUntypedPointerKHR)
return const_cast<SPIRVType *>(this);
assert(OpCode == OpTypePointer && "Not a pointer type");
return static_cast<const SPIRVTypePointer *>(this)->getElementType();
}

SPIRVStorageClassKind SPIRVType::getPointerStorageClass() const {
assert((OpCode == OpTypePointer || OpCode == OpTypeUntypedPointerKHR) &&
"Not a pointer type");
assert(OpCode == OpTypePointer && "Not a pointer type");
return static_cast<const SPIRVTypePointer *>(this)->getStorageClass();
}

Expand Down Expand Up @@ -187,13 +183,7 @@ bool SPIRVType::isTypeInt(unsigned Bits) const {
return isType<SPIRVTypeInt>(this, Bits);
}

bool SPIRVType::isTypePointer() const {
return OpCode == OpTypePointer || OpCode == OpTypeUntypedPointerKHR;
}

bool SPIRVType::isTypeUntypedPointerKHR() const {
return OpCode == OpTypeUntypedPointerKHR;
}
bool SPIRVType::isTypePointer() const { return OpCode == OpTypePointer; }

bool SPIRVType::isTypeOpaque() const { return OpCode == OpTypeOpaque; }

Expand Down
Loading

0 comments on commit b1f2327

Please sign in to comment.