Skip to content

Commit dadd5b3

Browse files
jsjijsji
committed
Revert "SPV_KHR_untyped_pointers - implement OpUntypedVariableKHR (#2709)"
This reverts commit 2f9d8d6.
1 parent 55ebb76 commit dadd5b3

File tree

15 files changed

+81
-336
lines changed

15 files changed

+81
-336
lines changed

‎llvm-spirv/lib/SPIRV/SPIRVReader.cpp

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,15 +1529,9 @@ Value *SPIRVToLLVM::transValueWithoutDecoration(SPIRVValue *BV, Function *F,
15291529
case OpUndef:
15301530
return mapValue(BV, UndefValue::get(transType(BV->getType())));
15311531

1532-
case OpVariable:
1533-
case OpUntypedVariableKHR: {
1534-
auto *BVar = static_cast<SPIRVVariableBase *>(BV);
1535-
SPIRVType *PreTransTy = BVar->getType()->getPointerElementType();
1536-
if (BVar->getType()->isTypeUntypedPointerKHR()) {
1537-
auto *UntypedVar = static_cast<SPIRVUntypedVariableKHR *>(BVar);
1538-
if (SPIRVType *DT = UntypedVar->getDataType())
1539-
PreTransTy = DT;
1540-
}
1532+
case OpVariable: {
1533+
auto *BVar = static_cast<SPIRVVariable *>(BV);
1534+
auto *PreTransTy = BVar->getType()->getPointerElementType();
15411535
auto *Ty = transType(PreTransTy);
15421536
bool IsConst = BVar->isConstant();
15431537
llvm::GlobalValue::LinkageTypes LinkageTy = transLinkageType(BVar);
@@ -4068,7 +4062,7 @@ bool SPIRVToLLVM::transDecoration(SPIRVValue *BV, Value *V) {
40684062
return true;
40694063
}
40704064

4071-
void SPIRVToLLVM::transGlobalCtorDtors(SPIRVVariableBase *BV) {
4065+
void SPIRVToLLVM::transGlobalCtorDtors(SPIRVVariable *BV) {
40724066
if (BV->getName() != "llvm.global_ctors" &&
40734067
BV->getName() != "llvm.global_dtors")
40744068
return;
@@ -4913,17 +4907,15 @@ SPIRVToLLVM::transLinkageType(const SPIRVValue *V) {
49134907
return GlobalValue::ExternalLinkage;
49144908
}
49154909
// Variable declaration
4916-
if (V->getOpCode() == OpVariable ||
4917-
V->getOpCode() == OpUntypedVariableKHR) {
4918-
if (static_cast<const SPIRVVariableBase *>(V)->getInitializer() == 0)
4910+
if (V->getOpCode() == OpVariable) {
4911+
if (static_cast<const SPIRVVariable *>(V)->getInitializer() == 0)
49194912
return GlobalValue::ExternalLinkage;
49204913
}
49214914
// Definition
49224915
return GlobalValue::AvailableExternallyLinkage;
49234916
case LinkageTypeExport:
4924-
if (V->getOpCode() == OpVariable ||
4925-
V->getOpCode() == OpUntypedVariableKHR) {
4926-
if (static_cast<const SPIRVVariableBase *>(V)->getInitializer() == 0)
4917+
if (V->getOpCode() == OpVariable) {
4918+
if (static_cast<const SPIRVVariable *>(V)->getInitializer() == 0)
49274919
// Tentative definition
49284920
return GlobalValue::CommonLinkage;
49294921
}

‎llvm-spirv/lib/SPIRV/SPIRVReader.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ class SPIRVToLLVM : private BuiltinCallHelper {
251251

252252
void transUserSemantic(SPIRV::SPIRVFunction *Fun);
253253
void transGlobalAnnotations();
254-
void transGlobalCtorDtors(SPIRVVariableBase *BV);
254+
void transGlobalCtorDtors(SPIRVVariable *BV);
255255
void createCXXStructor(const char *ListName,
256256
SmallVectorImpl<Function *> &Funcs);
257257
void transIntelFPGADecorations(SPIRVValue *BV, Value *V);

‎llvm-spirv/lib/SPIRV/SPIRVWriter.cpp

Lines changed: 24 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -755,8 +755,8 @@ SPIRVType *LLVMToSPIRVBase::transPointerType(SPIRVType *ET, unsigned AddrSpc) {
755755

756756
SPIRVType *TranslatedTy = nullptr;
757757
if (BM->isAllowedToUseExtension(ExtensionID::SPV_KHR_untyped_pointers) &&
758-
!(ET->isTypeArray() || ET->isTypeVector() || ET->isTypeStruct() ||
759-
ET->isTypeImage() || ET->isTypeSampler() || ET->isTypePipe())) {
758+
!(ET->isTypeArray() || ET->isTypeVector() || ET->isTypeImage() ||
759+
ET->isTypeSampler() || ET->isTypePipe())) {
760760
TranslatedTy = BM->addUntypedPointerKHRType(
761761
SPIRSPIRVAddrSpaceMap::map(static_cast<SPIRAddressSpace>(AddrSpc)));
762762
} else {
@@ -1329,8 +1329,7 @@ SPIRVValue *LLVMToSPIRVBase::transConstantUse(Constant *C,
13291329
if (Trans->getType() == ExpectedType || Trans->getType()->isTypePipeStorage())
13301330
return Trans;
13311331

1332-
assert((C->getType()->isPointerTy() ||
1333-
ExpectedType->isTypeUntypedPointerKHR()) &&
1332+
assert(C->getType()->isPointerTy() &&
13341333
"Only pointer type mismatches should be possible");
13351334
// In the common case of strings ([N x i8] GVs), see if we can emit a GEP
13361335
// instruction.
@@ -2068,12 +2067,8 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
20682067
}
20692068
}
20702069
}
2071-
if (BM->isAllowedToUseExtension(ExtensionID::SPV_KHR_untyped_pointers)) {
2072-
BVarInit = transConstantUse(Init, transType(Init->getType()));
2073-
} else {
2074-
SPIRVType *TransTy = transType(Ty);
2075-
BVarInit = transConstantUse(Init, TransTy->getPointerElementType());
2076-
}
2070+
SPIRVType *TransTy = transType(Ty);
2071+
BVarInit = transConstantUse(Init, TransTy->getPointerElementType());
20772072
}
20782073

20792074
SPIRVStorageClassKind StorageClass;
@@ -2106,12 +2101,9 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
21062101
}
21072102

21082103
SPIRVType *TranslatedTy = transType(Ty);
2109-
auto *BVar = static_cast<SPIRVVariableBase *>(BM->addVariable(
2110-
TranslatedTy,
2111-
TranslatedTy->isTypeUntypedPointerKHR() ? transType(GV->getValueType())
2112-
: nullptr,
2113-
GV->isConstant(), transLinkageType(GV), BVarInit, GV->getName().str(),
2114-
StorageClass, nullptr));
2104+
auto *BVar = static_cast<SPIRVVariable *>(
2105+
BM->addVariable(TranslatedTy, GV->isConstant(), transLinkageType(GV),
2106+
BVarInit, GV->getName().str(), StorageClass, nullptr));
21152107

21162108
if (IsVectorCompute) {
21172109
BVar->addDecorate(DecorationVectorComputeVariableINTEL);
@@ -2300,9 +2292,8 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
23002292
StorageClassFunction,
23012293
BM->addArrayType(transType(Alc->getAllocatedType()), Length));
23022294
SPIRVValue *Arr = BM->addVariable(
2303-
AllocationType, nullptr, false, spv::internal::LinkageTypeInternal,
2304-
nullptr, Alc->getName().str() + "_alloca", StorageClassFunction,
2305-
BB);
2295+
AllocationType, false, spv::internal::LinkageTypeInternal, nullptr,
2296+
Alc->getName().str() + "_alloca", StorageClassFunction, BB);
23062297
// Manually set alignment. OpBitcast created below will be decorated as
23072298
// that's the SPIR-V value mapped to the original LLVM one.
23082299
transAlign(Alc, Arr);
@@ -2326,10 +2317,7 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
23262317
TranslatedTy->getPointerElementType())
23272318
: TranslatedTy;
23282319
SPIRVValue *Var = BM->addVariable(
2329-
VarTy,
2330-
VarTy->isTypeUntypedPointerKHR() ? transType(Alc->getAllocatedType())
2331-
: nullptr,
2332-
false, spv::internal::LinkageTypeInternal, nullptr,
2320+
VarTy, false, spv::internal::LinkageTypeInternal, nullptr,
23332321
Alc->getName().str(), StorageClassFunction, BB);
23342322
if (V->getType()->getPointerAddressSpace() == SPIRAS_Generic) {
23352323
SPIRVValue *Cast =
@@ -2774,7 +2762,7 @@ void checkIsGlobalVar(SPIRVEntry *E, Decoration Dec) {
27742762
E->getErrorLog().checkError(E->isVariable(), SPIRVEC_InvalidModule, ErrStr);
27752763

27762764
auto AddrSpace = SPIRSPIRVAddrSpaceMap::rmap(
2777-
static_cast<SPIRVVariableBase *>(E)->getStorageClass());
2765+
static_cast<SPIRVVariable *>(E)->getStorageClass());
27782766
ErrStr += " in a global (module) scope";
27792767
E->getErrorLog().checkError(AddrSpace == SPIRAS_Global, SPIRVEC_InvalidModule,
27802768
ErrStr);
@@ -2922,11 +2910,10 @@ static void transMetadataDecorations(Metadata *MD, SPIRVValue *Target) {
29222910
case spv::internal::DecorationInitModeINTEL:
29232911
case DecorationInitModeINTEL: {
29242912
checkIsGlobalVar(Target, DecoKind);
2925-
ErrLog.checkError(
2926-
static_cast<SPIRVVariableBase *>(Target)->getInitializer(),
2927-
SPIRVEC_InvalidLlvmModule,
2928-
"InitModeINTEL only be applied to a global (module "
2929-
"scope) variable which has an Initializer operand");
2913+
ErrLog.checkError(static_cast<SPIRVVariable *>(Target)->getInitializer(),
2914+
SPIRVEC_InvalidLlvmModule,
2915+
"InitModeINTEL only be applied to a global (module "
2916+
"scope) variable which has an Initializer operand");
29302917

29312918
ErrLog.checkError(NumOperands == 2, SPIRVEC_InvalidLlvmModule,
29322919
"InitModeINTEL requires exactly 1 extra operand");
@@ -4168,18 +4155,14 @@ SPIRVValue *LLVMToSPIRVBase::transIntrinsicInst(IntrinsicInst *II,
41684155
SPIRVType *FTy = transType(II->getType()->getStructElementType(0));
41694156
SPIRVTypePointer *ITy = static_cast<SPIRVTypePointer *>(transPointerType(
41704157
II->getType()->getStructElementType(1), SPIRAS_Private));
4171-
if (!ITy->isTypeUntypedPointerKHR()) {
4172-
unsigned BitWidth = ITy->getElementType()->getBitWidth();
4173-
BM->getErrorLog().checkError(BitWidth == 32, SPIRVEC_InvalidBitWidth,
4174-
std::to_string(BitWidth));
4175-
}
4158+
4159+
unsigned BitWidth = ITy->getElementType()->getBitWidth();
4160+
BM->getErrorLog().checkError(BitWidth == 32, SPIRVEC_InvalidBitWidth,
4161+
std::to_string(BitWidth));
4162+
41764163
SPIRVValue *IntVal =
4177-
BM->addVariable(ITy,
4178-
ITy->isTypeUntypedPointerKHR()
4179-
? transType(II->getType()->getStructElementType(1))
4180-
: nullptr,
4181-
false, spv::internal::LinkageTypeInternal, nullptr, "",
4182-
ITy->getStorageClass(), BB);
4164+
BM->addVariable(ITy, false, spv::internal::LinkageTypeInternal, nullptr,
4165+
"", ITy->getStorageClass(), BB);
41834166

41844167
std::vector<SPIRVValue *> Ops{transValue(II->getArgOperand(0), BB), IntVal};
41854168

@@ -4601,7 +4584,7 @@ SPIRVValue *LLVMToSPIRVBase::transIntrinsicInst(IntrinsicInst *II,
46014584
Init = BM->addCompositeConstant(CompositeTy, Elts);
46024585
}
46034586
SPIRVType *VarTy = transPointerType(AT, SPIRV::SPIRAS_Constant);
4604-
SPIRVValue *Var = BM->addVariable(VarTy, nullptr, /*isConstant*/ true,
4587+
SPIRVValue *Var = BM->addVariable(VarTy, /*isConstant*/ true,
46054588
spv::internal::LinkageTypeInternal, Init,
46064589
"", StorageClassUniformConstant, nullptr);
46074590
SPIRVType *SourceTy =

‎llvm-spirv/lib/SPIRV/libSPIRV/SPIRVBasicBlock.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ SPIRVInstruction *SPIRVBasicBlock::getVariableInsertionPoint() const {
9494
isa<OpNoLine>(Inst) ||
9595
// Note: OpVariable and OpPhi instructions do not belong to the
9696
// same block in a valid SPIR-V module.
97-
isa<OpPhi>(Inst) || isa<OpUntypedVariableKHR>(Inst));
97+
isa<OpPhi>(Inst));
9898
});
9999
if (IP == InstVec.end())
100100
return nullptr;

‎llvm-spirv/lib/SPIRV/libSPIRV/SPIRVBasicBlock.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@ class SPIRVBasicBlock : public SPIRVValue {
7777
const SPIRVInstruction *getTerminateInstr() const {
7878
return InstVec.empty() ? nullptr : InstVec.back();
7979
}
80-
// Variables must be the first instructions in the block,
80+
// OpVariable instructions must be the first instructions in the block,
8181
// intermixed with OpLine and OpNoLine instructions. Return first instruction
82-
// not being an OpVariable, OpUntypedVariableKHR, OpLine or OpNoLine.
82+
// not being an OpVariable, OpLine or OpNoLine.
8383
SPIRVInstruction *getVariableInsertionPoint() const;
8484

8585
void setScope(SPIRVEntry *Scope) override;

‎llvm-spirv/lib/SPIRV/libSPIRV/SPIRVEntry.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -559,8 +559,7 @@ SPIRVEntry::getDecorationIds(Decoration Kind) const {
559559
}
560560

561561
bool SPIRVEntry::hasLinkageType() const {
562-
return OpCode == OpFunction || OpCode == OpVariable ||
563-
OpCode == OpUntypedVariableKHR;
562+
return OpCode == OpFunction || OpCode == OpVariable;
564563
}
565564

566565
bool SPIRVEntry::isExtInst(const SPIRVExtInstSetKind InstSet) const {

‎llvm-spirv/lib/SPIRV/libSPIRV/SPIRVEntry.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,7 @@ class SPIRVEntry {
339339
bool isUndef() const { return OpCode == OpUndef; }
340340
bool isControlBarrier() const { return OpCode == OpControlBarrier; }
341341
bool isMemoryBarrier() const { return OpCode == OpMemoryBarrier; }
342-
bool isVariable() const {
343-
return OpCode == OpVariable || OpCode == OpUntypedVariableKHR;
344-
}
342+
bool isVariable() const { return OpCode == OpVariable; }
345343
bool isEndOfBlock() const;
346344
virtual bool isInst() const { return false; }
347345
virtual bool isOperandLiteral(unsigned Index) const {

‎llvm-spirv/lib/SPIRV/libSPIRV/SPIRVInstruction.h

Lines changed: 18 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -452,25 +452,24 @@ class SPIRVMemoryAccess {
452452
SPIRVId NoAliasInstID;
453453
};
454454

455-
class SPIRVVariableBase : public SPIRVInstruction {
455+
class SPIRVVariable : public SPIRVInstruction {
456456
public:
457457
// Complete constructor for integer constant
458-
SPIRVVariableBase(Op OC, SPIRVType *TheType, SPIRVId TheId,
459-
SPIRVValue *TheInitializer, const std::string &TheName,
460-
SPIRVStorageClassKind TheStorageClass,
461-
SPIRVBasicBlock *TheBB, SPIRVModule *TheM, SPIRVWord WC)
462-
: SPIRVInstruction(WC, OC, TheType, TheId, TheBB, TheM),
458+
SPIRVVariable(SPIRVType *TheType, SPIRVId TheId, SPIRVValue *TheInitializer,
459+
const std::string &TheName,
460+
SPIRVStorageClassKind TheStorageClass, SPIRVBasicBlock *TheBB,
461+
SPIRVModule *TheM)
462+
: SPIRVInstruction(TheInitializer && !TheInitializer->isUndef() ? 5 : 4,
463+
OpVariable, TheType, TheId, TheBB, TheM),
463464
StorageClass(TheStorageClass) {
464465
if (TheInitializer && !TheInitializer->isUndef())
465466
Initializer.push_back(TheInitializer->getId());
466467
Name = TheName;
467468
validate();
468469
}
469-
// Incomplete constructors
470-
SPIRVVariableBase(Op OC)
471-
: SPIRVInstruction(OC), StorageClass(StorageClassFunction) {}
472-
SPIRVVariableBase()
473-
: SPIRVInstruction(OpNop), StorageClass(StorageClassFunction) {}
470+
// Incomplete constructor
471+
SPIRVVariable()
472+
: SPIRVInstruction(OpVariable), StorageClass(StorageClassFunction) {}
474473

475474
SPIRVStorageClassKind getStorageClass() const { return StorageClass; }
476475
SPIRVValue *getInitializer() const {
@@ -522,77 +521,6 @@ class SPIRVVariableBase : public SPIRVInstruction {
522521
std::vector<SPIRVId> Initializer;
523522
};
524523

525-
class SPIRVVariable : public SPIRVVariableBase {
526-
public:
527-
// Complete constructor for integer constant
528-
SPIRVVariable(SPIRVType *TheType, SPIRVId TheId, SPIRVValue *TheInitializer,
529-
const std::string &TheName,
530-
SPIRVStorageClassKind TheStorageClass, SPIRVBasicBlock *TheBB,
531-
SPIRVModule *TheM)
532-
: SPIRVVariableBase(OpVariable, TheType, TheId, TheInitializer, TheName,
533-
TheStorageClass, TheBB, TheM,
534-
TheInitializer && !TheInitializer->isUndef() ? 5
535-
: 4) {}
536-
// Incomplete constructor
537-
SPIRVVariable() : SPIRVVariableBase(OpVariable) {}
538-
};
539-
540-
class SPIRVUntypedVariableKHR : public SPIRVVariableBase {
541-
public:
542-
SPIRVUntypedVariableKHR(SPIRVType *TheType, SPIRVId TheId,
543-
SPIRVType *TheDataType, SPIRVValue *TheInitializer,
544-
const std::string &TheName,
545-
SPIRVStorageClassKind TheStorageClass,
546-
SPIRVBasicBlock *TheBB, SPIRVModule *TheM)
547-
: SPIRVVariableBase(
548-
OpUntypedVariableKHR, TheType, TheId, TheInitializer, TheName,
549-
TheStorageClass, TheBB, TheM,
550-
TheDataType && !TheDataType->isUndef()
551-
? (TheInitializer && !TheInitializer->isUndef() ? 6 : 5)
552-
: 4) {
553-
if (TheDataType && !TheDataType->isUndef())
554-
DataType.push_back(TheDataType->getId());
555-
validate();
556-
}
557-
SPIRVUntypedVariableKHR() : SPIRVVariableBase(OpUntypedVariableKHR) {}
558-
SPIRVType *getDataType() const {
559-
if (DataType.empty())
560-
return nullptr;
561-
assert(DataType.size() == 1);
562-
return get<SPIRVType>(DataType[0]);
563-
}
564-
std::vector<SPIRVEntry *> getNonLiteralOperands() const override {
565-
std::vector<SPIRVEntry *> Vec;
566-
if (SPIRVType *T = getDataType())
567-
Vec.push_back(T);
568-
if (SPIRVValue *V = getInitializer())
569-
Vec.push_back(V);
570-
return Vec;
571-
}
572-
std::optional<ExtensionID> getRequiredExtension() const override {
573-
return ExtensionID::SPV_KHR_untyped_pointers;
574-
}
575-
SPIRVCapVec getRequiredCapability() const override {
576-
return getVec(CapabilityUntypedPointersKHR);
577-
}
578-
579-
protected:
580-
void validate() const override {
581-
SPIRVVariableBase::validate();
582-
assert(DataType.size() == 1 || DataType.empty());
583-
}
584-
void setWordCount(SPIRVWord TheWordCount) override {
585-
SPIRVEntry::setWordCount(TheWordCount);
586-
if (TheWordCount > 4)
587-
DataType.resize(1);
588-
if (TheWordCount > 5)
589-
Initializer.resize(1);
590-
}
591-
_SPIRV_DEF_ENCDEC5(Type, Id, StorageClass, DataType, Initializer)
592-
593-
std::vector<SPIRVId> DataType;
594-
};
595-
596524
class SPIRVStore : public SPIRVInstruction, public SPIRVMemoryAccess {
597525
public:
598526
const static SPIRVWord FixedWords = 3;
@@ -644,6 +572,9 @@ class SPIRVStore : public SPIRVInstruction, public SPIRVMemoryAccess {
644572
(getValueType(PtrId)
645573
->getPointerElementType()
646574
->isTypeUntypedPointerKHR() ||
575+
// TODO: This check should be removed once we support untyped
576+
// variables.
577+
getValueType(ValId)->isTypeUntypedPointerKHR() ||
647578
getValueType(PtrId)->getPointerElementType() == getValueType(ValId)) &&
648579
"Inconsistent operand types");
649580
}
@@ -698,6 +629,8 @@ class SPIRVLoad : public SPIRVInstruction, public SPIRVMemoryAccess {
698629
getValueType(PtrId)
699630
->getPointerElementType()
700631
->isTypeUntypedPointerKHR() ||
632+
// TODO: This check should be removed once we support untyped
633+
// variables.
701634
Type->isTypeUntypedPointerKHR() ||
702635
Type == getValueType(PtrId)->getPointerElementType()) &&
703636
"Inconsistent types");
@@ -755,14 +688,9 @@ class SPIRVBinary : public SPIRVInstTemplateBase {
755688
} else if (isBinaryPtrOpCode(OpCode)) {
756689
assert((Op1Ty->isTypePointer() && Op2Ty->isTypePointer()) &&
757690
"Invalid types for PtrEqual, PtrNotEqual, or PtrDiff instruction");
758-
if (!Op1Ty->isTypeUntypedPointerKHR() ||
759-
!Op2Ty->isTypeUntypedPointerKHR())
760-
assert(
761-
static_cast<SPIRVTypePointer *>(Op1Ty)->getElementType() ==
762-
static_cast<SPIRVTypePointer *>(Op2Ty)->getElementType() &&
763-
"Invalid types for PtrEqual, PtrNotEqual, or PtrDiff instruction");
764-
else if (OpCode == OpPtrDiff)
765-
assert(Op1Ty == Op2Ty && "Invalid types for PtrDiff instruction");
691+
assert(static_cast<SPIRVTypePointer *>(Op1Ty)->getElementType() ==
692+
static_cast<SPIRVTypePointer *>(Op2Ty)->getElementType() &&
693+
"Invalid types for PtrEqual, PtrNotEqual, or PtrDiff instruction");
766694
} else {
767695
assert(0 && "Invalid op code!");
768696
}

0 commit comments

Comments
 (0)