Skip to content

Commit 42633cf

Browse files
VyacheslavLevytskyyVyacheslavLevytskyy
authored
[SPIR-V] Improve general validity of emitted code between passes (#119202)
This PR improves general validity of emitted code between passes due to generation of `TargetOpcode::PHI` instead of `SPIRV::OpPhi` after Instruction Selection, fixing generation of OpTypePointer instructions and using of proper virtual register classes. Using `TargetOpcode::PHI` instead of `SPIRV::OpPhi` after Instruction Selection has a benefit to support existing optimization passes immediately, as an alternative path to disable those passes that use `MI.isPHI()`. This PR makes it possible thus to revert #116060 actions and get back to use the `MachineSink` pass. This PR is a solution of the problem discussed in details in #110507. It accepts an advice from code reviewers of the PR #110507 to postpone generation of OpPhi rather than to patch CodeGen. This solution allows to unblock improvements wrt. expensive checks and makes it unrelated to the general points of the discussion about OpPhi vs. G_PHI/PHI. This PR contains numerous small patches of emitted code validity that allows to substantially pass rate with expensive checks. Namely, the test suite with expensive checks set ON now has only 12 fails out of 569 total test cases. FYI @bogner
1 parent 0f7b3a9 commit 42633cf

17 files changed

+228
-198
lines changed

‎llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,8 +1633,14 @@ static bool generateICarryBorrowInst(const SPIRV::IncomingCall *Call,
16331633
}
16341634

16351635
MachineRegisterInfo *MRI = MIRBuilder.getMRI();
1636-
Register ResReg = MRI->createGenericVirtualRegister(LLT::scalar(64));
1637-
MRI->setRegClass(ResReg, &SPIRV::iIDRegClass);
1636+
Register ResReg = MRI->createVirtualRegister(&SPIRV::iIDRegClass);
1637+
if (const TargetRegisterClass *DstRC =
1638+
MRI->getRegClassOrNull(Call->Arguments[1])) {
1639+
MRI->setRegClass(ResReg, DstRC);
1640+
MRI->setType(ResReg, MRI->getType(Call->Arguments[1]));
1641+
} else {
1642+
MRI->setType(ResReg, LLT::scalar(64));
1643+
}
16381644
GR->assignSPIRVTypeToVReg(RetType, ResReg, MIRBuilder.getMF());
16391645
MIRBuilder.buildInstr(Opcode)
16401646
.addDef(ResReg)

‎llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp‎

Lines changed: 129 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ inline Register createTypeVReg(MachineIRBuilder &MIRBuilder) {
9696
}
9797

9898
SPIRVType *SPIRVGlobalRegistry::getOpTypeBool(MachineIRBuilder &MIRBuilder) {
99-
10099
return createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
101100
return MIRBuilder.buildInstr(SPIRV::OpTypeBool)
102101
.addDef(createTypeVReg(MIRBuilder));
@@ -166,8 +165,11 @@ SPIRVType *SPIRVGlobalRegistry::createOpType(
166165

167166
auto LastInsertedType = LastInsertedTypeMap.find(CurMF);
168167
if (LastInsertedType != LastInsertedTypeMap.end()) {
169-
MIRBuilder.setInsertPt(*MIRBuilder.getMF().begin(),
170-
LastInsertedType->second->getIterator());
168+
auto It = LastInsertedType->second->getIterator();
169+
auto NewMBB = MIRBuilder.getMF().begin();
170+
MIRBuilder.setInsertPt(*NewMBB, It->getNextNode()
171+
? It->getNextNode()->getIterator()
172+
: NewMBB->end());
171173
} else {
172174
MIRBuilder.setInsertPt(*MIRBuilder.getMF().begin(),
173175
MIRBuilder.getMF().begin()->begin());
@@ -269,24 +271,27 @@ Register SPIRVGlobalRegistry::getOrCreateConstFP(APFloat Val, MachineInstr &I,
269271
// machine instruction, a new constant instruction should be created.
270272
if (!New && (!I.getOperand(0).isReg() || Res != I.getOperand(0).getReg()))
271273
return Res;
272-
MachineInstrBuilder MIB;
273-
MachineBasicBlock &BB = *I.getParent();
274-
// In OpenCL OpConstantNull - Scalar floating point: +0.0 (all bits 0)
275-
if (Val.isPosZero() && ZeroAsNull) {
276-
MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpConstantNull))
277-
.addDef(Res)
278-
.addUse(getSPIRVTypeID(SpvType));
279-
} else {
280-
MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpConstantF))
281-
.addDef(Res)
282-
.addUse(getSPIRVTypeID(SpvType));
283-
addNumImm(
284-
APInt(BitWidth, CI->getValueAPF().bitcastToAPInt().getZExtValue()),
285-
MIB);
286-
}
287-
const auto &ST = CurMF->getSubtarget();
288-
constrainSelectedInstRegOperands(*MIB, *ST.getInstrInfo(),
289-
*ST.getRegisterInfo(), *ST.getRegBankInfo());
274+
MachineIRBuilder MIRBuilder(I);
275+
createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
276+
MachineInstrBuilder MIB;
277+
// In OpenCL OpConstantNull - Scalar floating point: +0.0 (all bits 0)
278+
if (Val.isPosZero() && ZeroAsNull) {
279+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantNull)
280+
.addDef(Res)
281+
.addUse(getSPIRVTypeID(SpvType));
282+
} else {
283+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantF)
284+
.addDef(Res)
285+
.addUse(getSPIRVTypeID(SpvType));
286+
addNumImm(
287+
APInt(BitWidth, CI->getValueAPF().bitcastToAPInt().getZExtValue()),
288+
MIB);
289+
}
290+
const auto &ST = CurMF->getSubtarget();
291+
constrainSelectedInstRegOperands(
292+
*MIB, *ST.getInstrInfo(), *ST.getRegisterInfo(), *ST.getRegBankInfo());
293+
return MIB;
294+
});
290295
return Res;
291296
}
292297

@@ -305,21 +310,25 @@ Register SPIRVGlobalRegistry::getOrCreateConstInt(uint64_t Val, MachineInstr &I,
305310
// machine instruction, a new constant instruction should be created.
306311
if (!New && (!I.getOperand(0).isReg() || Res != I.getOperand(0).getReg()))
307312
return Res;
308-
MachineInstrBuilder MIB;
309-
MachineBasicBlock &BB = *I.getParent();
310-
if (Val || !ZeroAsNull) {
311-
MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpConstantI))
312-
.addDef(Res)
313-
.addUse(getSPIRVTypeID(SpvType));
314-
addNumImm(APInt(BitWidth, Val), MIB);
315-
} else {
316-
MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpConstantNull))
317-
.addDef(Res)
318-
.addUse(getSPIRVTypeID(SpvType));
319-
}
320-
const auto &ST = CurMF->getSubtarget();
321-
constrainSelectedInstRegOperands(*MIB, *ST.getInstrInfo(),
322-
*ST.getRegisterInfo(), *ST.getRegBankInfo());
313+
314+
MachineIRBuilder MIRBuilder(I);
315+
createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
316+
MachineInstrBuilder MIB;
317+
if (Val || !ZeroAsNull) {
318+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantI)
319+
.addDef(Res)
320+
.addUse(getSPIRVTypeID(SpvType));
321+
addNumImm(APInt(BitWidth, Val), MIB);
322+
} else {
323+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantNull)
324+
.addDef(Res)
325+
.addUse(getSPIRVTypeID(SpvType));
326+
}
327+
const auto &ST = CurMF->getSubtarget();
328+
constrainSelectedInstRegOperands(
329+
*MIB, *ST.getInstrInfo(), *ST.getRegisterInfo(), *ST.getRegBankInfo());
330+
return MIB;
331+
});
323332
return Res;
324333
}
325334

@@ -347,21 +356,24 @@ Register SPIRVGlobalRegistry::buildConstantInt(uint64_t Val,
347356
MIRBuilder.buildConstant(Res, *ConstInt);
348357
} else {
349358
Register SpvTypeReg = getSPIRVTypeID(SpvType);
350-
MachineInstrBuilder MIB;
351-
if (Val || !ZeroAsNull) {
352-
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantI)
353-
.addDef(Res)
354-
.addUse(SpvTypeReg);
355-
addNumImm(APInt(BitWidth, Val), MIB);
356-
} else {
357-
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantNull)
358-
.addDef(Res)
359-
.addUse(SpvTypeReg);
360-
}
361-
const auto &Subtarget = CurMF->getSubtarget();
362-
constrainSelectedInstRegOperands(*MIB, *Subtarget.getInstrInfo(),
363-
*Subtarget.getRegisterInfo(),
364-
*Subtarget.getRegBankInfo());
359+
createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
360+
MachineInstrBuilder MIB;
361+
if (Val || !ZeroAsNull) {
362+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantI)
363+
.addDef(Res)
364+
.addUse(SpvTypeReg);
365+
addNumImm(APInt(BitWidth, Val), MIB);
366+
} else {
367+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantNull)
368+
.addDef(Res)
369+
.addUse(SpvTypeReg);
370+
}
371+
const auto &Subtarget = CurMF->getSubtarget();
372+
constrainSelectedInstRegOperands(*MIB, *Subtarget.getInstrInfo(),
373+
*Subtarget.getRegisterInfo(),
374+
*Subtarget.getRegBankInfo());
375+
return MIB;
376+
});
365377
}
366378
}
367379
return Res;
@@ -385,12 +397,14 @@ Register SPIRVGlobalRegistry::buildConstantFP(APFloat Val,
385397
MF.getRegInfo().setRegClass(Res, &SPIRV::fIDRegClass);
386398
assignSPIRVTypeToVReg(SpvType, Res, MF);
387399
DT.add(ConstFP, &MF, Res);
388-
389-
MachineInstrBuilder MIB;
390-
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantF)
391-
.addDef(Res)
392-
.addUse(getSPIRVTypeID(SpvType));
393-
addNumImm(ConstFP->getValueAPF().bitcastToAPInt(), MIB);
400+
createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
401+
MachineInstrBuilder MIB;
402+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantF)
403+
.addDef(Res)
404+
.addUse(getSPIRVTypeID(SpvType));
405+
addNumImm(ConstFP->getValueAPF().bitcastToAPInt(), MIB);
406+
return MIB;
407+
});
394408
}
395409

396410
return Res;
@@ -439,23 +453,26 @@ Register SPIRVGlobalRegistry::getOrCreateCompositeOrNull(
439453
CurMF->getRegInfo().setRegClass(SpvVecConst, getRegClass(SpvType));
440454
assignSPIRVTypeToVReg(SpvType, SpvVecConst, *CurMF);
441455
DT.add(CA, CurMF, SpvVecConst);
442-
MachineInstrBuilder MIB;
443-
MachineBasicBlock &BB = *I.getParent();
444-
if (!IsNull) {
445-
MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpConstantComposite))
446-
.addDef(SpvVecConst)
447-
.addUse(getSPIRVTypeID(SpvType));
448-
for (unsigned i = 0; i < ElemCnt; ++i)
449-
MIB.addUse(SpvScalConst);
450-
} else {
451-
MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpConstantNull))
452-
.addDef(SpvVecConst)
453-
.addUse(getSPIRVTypeID(SpvType));
454-
}
455-
const auto &Subtarget = CurMF->getSubtarget();
456-
constrainSelectedInstRegOperands(*MIB, *Subtarget.getInstrInfo(),
457-
*Subtarget.getRegisterInfo(),
458-
*Subtarget.getRegBankInfo());
456+
MachineIRBuilder MIRBuilder(I);
457+
createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
458+
MachineInstrBuilder MIB;
459+
if (!IsNull) {
460+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantComposite)
461+
.addDef(SpvVecConst)
462+
.addUse(getSPIRVTypeID(SpvType));
463+
for (unsigned i = 0; i < ElemCnt; ++i)
464+
MIB.addUse(SpvScalConst);
465+
} else {
466+
MIB = MIRBuilder.buildInstr(SPIRV::OpConstantNull)
467+
.addDef(SpvVecConst)
468+
.addUse(getSPIRVTypeID(SpvType));
469+
}
470+
const auto &Subtarget = CurMF->getSubtarget();
471+
constrainSelectedInstRegOperands(*MIB, *Subtarget.getInstrInfo(),
472+
*Subtarget.getRegisterInfo(),
473+
*Subtarget.getRegBankInfo());
474+
return MIB;
475+
});
459476
return SpvVecConst;
460477
}
461478
return Res;
@@ -544,17 +561,20 @@ Register SPIRVGlobalRegistry::getOrCreateIntCompositeOrNull(
544561
if (EmitIR) {
545562
MIRBuilder.buildSplatBuildVector(SpvVecConst, SpvScalConst);
546563
} else {
547-
if (Val) {
548-
auto MIB = MIRBuilder.buildInstr(SPIRV::OpConstantComposite)
549-
.addDef(SpvVecConst)
550-
.addUse(getSPIRVTypeID(SpvType));
551-
for (unsigned i = 0; i < ElemCnt; ++i)
552-
MIB.addUse(SpvScalConst);
553-
} else {
554-
MIRBuilder.buildInstr(SPIRV::OpConstantNull)
555-
.addDef(SpvVecConst)
556-
.addUse(getSPIRVTypeID(SpvType));
557-
}
564+
createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
565+
if (Val) {
566+
auto MIB = MIRBuilder.buildInstr(SPIRV::OpConstantComposite)
567+
.addDef(SpvVecConst)
568+
.addUse(getSPIRVTypeID(SpvType));
569+
for (unsigned i = 0; i < ElemCnt; ++i)
570+
MIB.addUse(SpvScalConst);
571+
return MIB;
572+
} else {
573+
return MIRBuilder.buildInstr(SPIRV::OpConstantNull)
574+
.addDef(SpvVecConst)
575+
.addUse(getSPIRVTypeID(SpvType));
576+
}
577+
});
558578
}
559579
return SpvVecConst;
560580
}
@@ -592,9 +612,11 @@ SPIRVGlobalRegistry::getOrCreateConstNullPtr(MachineIRBuilder &MIRBuilder,
592612
Res = CurMF->getRegInfo().createGenericVirtualRegister(LLTy);
593613
CurMF->getRegInfo().setRegClass(Res, &SPIRV::pIDRegClass);
594614
assignSPIRVTypeToVReg(SpvType, Res, *CurMF);
595-
MIRBuilder.buildInstr(SPIRV::OpConstantNull)
596-
.addDef(Res)
597-
.addUse(getSPIRVTypeID(SpvType));
615+
createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
616+
return MIRBuilder.buildInstr(SPIRV::OpConstantNull)
617+
.addDef(Res)
618+
.addUse(getSPIRVTypeID(SpvType));
619+
});
598620
DT.add(CP, CurMF, Res);
599621
}
600622
return Res;
@@ -614,12 +636,14 @@ Register SPIRVGlobalRegistry::buildConstantSampler(
614636
ResReg.isValid()
615637
? ResReg
616638
: MIRBuilder.getMRI()->createVirtualRegister(&SPIRV::iIDRegClass);
617-
auto Res = MIRBuilder.buildInstr(SPIRV::OpConstantSampler)
618-
.addDef(Sampler)
619-
.addUse(getSPIRVTypeID(SampTy))
620-
.addImm(AddrMode)
621-
.addImm(Param)
622-
.addImm(FilerMode);
639+
auto Res = createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
640+
return MIRBuilder.buildInstr(SPIRV::OpConstantSampler)
641+
.addDef(Sampler)
642+
.addUse(getSPIRVTypeID(SampTy))
643+
.addImm(AddrMode)
644+
.addImm(Param)
645+
.addImm(FilerMode);
646+
});
623647
assert(Res->getOperand(0).isReg());
624648
return Res->getOperand(0).getReg();
625649
}
@@ -1551,14 +1575,17 @@ SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVPointerType(
15511575
if (Reg.isValid())
15521576
return getSPIRVTypeForVReg(Reg);
15531577
// create a new type
1554-
auto MIB = BuildMI(MIRBuilder.getMBB(), MIRBuilder.getInsertPt(),
1555-
MIRBuilder.getDebugLoc(),
1556-
MIRBuilder.getTII().get(SPIRV::OpTypePointer))
1557-
.addDef(createTypeVReg(CurMF->getRegInfo()))
1558-
.addImm(static_cast<uint32_t>(SC))
1559-
.addUse(getSPIRVTypeID(BaseType));
1560-
DT.add(PointerElementType, AddressSpace, CurMF, getSPIRVTypeID(MIB));
1561-
return finishCreatingSPIRVType(LLVMTy, MIB);
1578+
return createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
1579+
auto MIB = BuildMI(MIRBuilder.getMBB(), MIRBuilder.getInsertPt(),
1580+
MIRBuilder.getDebugLoc(),
1581+
MIRBuilder.getTII().get(SPIRV::OpTypePointer))
1582+
.addDef(createTypeVReg(CurMF->getRegInfo()))
1583+
.addImm(static_cast<uint32_t>(SC))
1584+
.addUse(getSPIRVTypeID(BaseType));
1585+
DT.add(PointerElementType, AddressSpace, CurMF, getSPIRVTypeID(MIB));
1586+
finishCreatingSPIRVType(LLVMTy, MIB);
1587+
return MIB;
1588+
});
15621589
}
15631590

15641591
SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVPointerType(

0 commit comments

Comments
 (0)