Skip to content

[SPIR-V] Fix illegal OpConstantComposite instruction with non-const constituents in SPIR-V Backend #86352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ void SPIRVGeneralDuplicatesTracker::buildDepsGraph(
prebuildReg2Entry(GT, Reg2Entry);
prebuildReg2Entry(FT, Reg2Entry);
prebuildReg2Entry(AT, Reg2Entry);
prebuildReg2Entry(MT, Reg2Entry);
prebuildReg2Entry(ST, Reg2Entry);

for (auto &Op2E : Reg2Entry) {
Expand Down
9 changes: 9 additions & 0 deletions llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ class SPIRVGeneralDuplicatesTracker {
SPIRVDuplicatesTracker<GlobalVariable> GT;
SPIRVDuplicatesTracker<Function> FT;
SPIRVDuplicatesTracker<Argument> AT;
SPIRVDuplicatesTracker<MachineInstr> MT;
SPIRVDuplicatesTracker<SPIRV::SpecialTypeDescriptor> ST;

// NOTE: using MOs instead of regs to get rid of MF dependency to be able
Expand Down Expand Up @@ -306,6 +307,10 @@ class SPIRVGeneralDuplicatesTracker {
AT.add(Arg, MF, R);
}

void add(const MachineInstr *MI, const MachineFunction *MF, Register R) {
MT.add(MI, MF, R);
}

void add(const SPIRV::SpecialTypeDescriptor &TD, const MachineFunction *MF,
Register R) {
ST.add(TD, MF, R);
Expand Down Expand Up @@ -337,6 +342,10 @@ class SPIRVGeneralDuplicatesTracker {
return AT.find(const_cast<Argument *>(Arg), MF);
}

Register find(const MachineInstr *MI, const MachineFunction *MF) {
return MT.find(const_cast<MachineInstr *>(MI), MF);
}

Register find(const SPIRV::SpecialTypeDescriptor &TD,
const MachineFunction *MF) {
return ST.find(TD, MF);
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ SPIRVType *SPIRVGlobalRegistry::getOpTypeVector(uint32_t NumElems,
SPIRVType *ElemType,
MachineIRBuilder &MIRBuilder) {
auto EleOpc = ElemType->getOpcode();
(void)EleOpc;
assert((EleOpc == SPIRV::OpTypeInt || EleOpc == SPIRV::OpTypeFloat ||
EleOpc == SPIRV::OpTypeBool) &&
"Invalid vector element type");
Expand Down
8 changes: 8 additions & 0 deletions llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ class SPIRVGlobalRegistry {
DT.add(Arg, MF, R);
}

void add(const MachineInstr *MI, MachineFunction *MF, Register R) {
DT.add(MI, MF, R);
}

Register find(const MachineInstr *MI, MachineFunction *MF) {
return DT.find(MI, MF);
}

Register find(const Constant *C, MachineFunction *MF) {
return DT.find(C, MF);
}
Expand Down
90 changes: 74 additions & 16 deletions llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ class SPIRVInstructionSelector : public InstructionSelector {
Register buildZerosVal(const SPIRVType *ResType, MachineInstr &I) const;
Register buildOnesVal(bool AllOnes, const SPIRVType *ResType,
MachineInstr &I) const;

bool wrapIntoSpecConstantOp(MachineInstr &I,
SmallVector<Register> &CompositeArgs) const;
};

} // end anonymous namespace
Expand Down Expand Up @@ -1245,6 +1248,24 @@ static unsigned getArrayComponentCount(MachineRegisterInfo *MRI,
return N;
}

// Return true if the type represents a constant register
static bool isConstReg(MachineRegisterInfo *MRI, SPIRVType *OpDef) {
if (OpDef->getOpcode() == SPIRV::ASSIGN_TYPE &&
OpDef->getOperand(1).isReg()) {
if (SPIRVType *RefDef = MRI->getVRegDef(OpDef->getOperand(1).getReg()))
OpDef = RefDef;
}
return OpDef->getOpcode() == TargetOpcode::G_CONSTANT ||
OpDef->getOpcode() == TargetOpcode::G_FCONSTANT;
}

// Return true if the virtual register represents a constant
static bool isConstReg(MachineRegisterInfo *MRI, Register OpReg) {
if (SPIRVType *OpDef = MRI->getVRegDef(OpReg))
return isConstReg(MRI, OpDef);
return false;
}

bool SPIRVInstructionSelector::selectSplatVector(Register ResVReg,
const SPIRVType *ResType,
MachineInstr &I) const {
Expand All @@ -1262,16 +1283,7 @@ bool SPIRVInstructionSelector::selectSplatVector(Register ResVReg,

// check if we may construct a constant vector
Register OpReg = I.getOperand(OpIdx).getReg();
bool IsConst = false;
if (SPIRVType *OpDef = MRI->getVRegDef(OpReg)) {
if (OpDef->getOpcode() == SPIRV::ASSIGN_TYPE &&
OpDef->getOperand(1).isReg()) {
if (SPIRVType *RefDef = MRI->getVRegDef(OpDef->getOperand(1).getReg()))
OpDef = RefDef;
}
IsConst = OpDef->getOpcode() == TargetOpcode::G_CONSTANT ||
OpDef->getOpcode() == TargetOpcode::G_FCONSTANT;
}
bool IsConst = isConstReg(MRI, OpReg);

if (!IsConst && N < 2)
report_fatal_error(
Expand Down Expand Up @@ -1624,6 +1636,48 @@ bool SPIRVInstructionSelector::selectGEP(Register ResVReg,
return Res.constrainAllUses(TII, TRI, RBI);
}

// Maybe wrap a value into OpSpecConstantOp
bool SPIRVInstructionSelector::wrapIntoSpecConstantOp(
MachineInstr &I, SmallVector<Register> &CompositeArgs) const {
bool Result = true;
unsigned Lim = I.getNumExplicitOperands();
for (unsigned i = I.getNumExplicitDefs() + 1; i < Lim; ++i) {
Register OpReg = I.getOperand(i).getReg();
SPIRVType *OpDefine = MRI->getVRegDef(OpReg);
SPIRVType *OpType = GR.getSPIRVTypeForVReg(OpReg);
if (!OpDefine || !OpType || isConstReg(MRI, OpDefine) ||
OpDefine->getOpcode() == TargetOpcode::G_ADDRSPACE_CAST) {
// The case of G_ADDRSPACE_CAST inside spv_const_composite() is processed
// by selectAddrSpaceCast()
CompositeArgs.push_back(OpReg);
continue;
}
MachineFunction *MF = I.getMF();
Register WrapReg = GR.find(OpDefine, MF);
if (WrapReg.isValid()) {
CompositeArgs.push_back(WrapReg);
continue;
}
// Create a new register for the wrapper
WrapReg = MRI->createVirtualRegister(&SPIRV::IDRegClass);
GR.add(OpDefine, MF, WrapReg);
CompositeArgs.push_back(WrapReg);
// Decorate the wrapper register and generate a new instruction
MRI->setType(WrapReg, LLT::pointer(0, 32));
GR.assignSPIRVTypeToVReg(OpType, WrapReg, *MF);
MachineBasicBlock &BB = *I.getParent();
Result = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpSpecConstantOp))
.addDef(WrapReg)
.addUse(GR.getSPIRVTypeID(OpType))
.addImm(static_cast<uint32_t>(SPIRV::Opcode::Bitcast))
.addUse(OpReg)
.constrainAllUses(TII, TRI, RBI);
if (!Result)
break;
}
return Result;
}

bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
const SPIRVType *ResType,
MachineInstr &I) const {
Expand Down Expand Up @@ -1662,17 +1716,21 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
case Intrinsic::spv_const_composite: {
// If no values are attached, the composite is null constant.
bool IsNull = I.getNumExplicitDefs() + 1 == I.getNumExplicitOperands();
unsigned Opcode =
IsNull ? SPIRV::OpConstantNull : SPIRV::OpConstantComposite;
// Select a proper instruction.
unsigned Opcode = SPIRV::OpConstantNull;
SmallVector<Register> CompositeArgs;
if (!IsNull) {
Opcode = SPIRV::OpConstantComposite;
if (!wrapIntoSpecConstantOp(I, CompositeArgs))
return false;
}
auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(Opcode))
.addDef(ResVReg)
.addUse(GR.getSPIRVTypeID(ResType));
// skip type MD node we already used when generated assign.type for this
if (!IsNull) {
for (unsigned i = I.getNumExplicitDefs() + 1;
i < I.getNumExplicitOperands(); ++i) {
MIB.addUse(I.getOperand(i).getReg());
}
for (Register OpReg : CompositeArgs)
MIB.addUse(OpReg);
}
return MIB.constrainAllUses(TII, TRI, RBI);
}
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
Register Dst = ICMP->getOperand(0).getReg();
MachineOperand &PredOp = ICMP->getOperand(1);
const auto CC = static_cast<CmpInst::Predicate>(PredOp.getPredicate());
(void)CC;
assert((CC == CmpInst::ICMP_EQ || CC == CmpInst::ICMP_ULE) &&
MRI.hasOneUse(Dst) && MRI.hasOneDef(CompareReg));
uint64_t Value = getIConstVal(ICMP->getOperand(3).getReg(), &MRI);
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td
Original file line number Diff line number Diff line change
Expand Up @@ -1611,3 +1611,4 @@ multiclass OpcodeOperand<bits<32> value> {
// TODO: implement other mnemonics.
defm InBoundsPtrAccessChain : OpcodeOperand<70>;
defm PtrCastToGeneric : OpcodeOperand<121>;
defm Bitcast : OpcodeOperand<124>;
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/SPIRV/pointers/struct-opaque-pointers.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}

; CHECK: %[[TyInt8:.*]] = OpTypeInt 8 0
; CHECK: %[[TyInt8Ptr:.*]] = OpTypePointer {{[a-zA-Z]+}} %[[TyInt8]]
Expand Down