Skip to content

Commit 7334a28

Browse files
committed
[SPIR-V][Codegen] Add isPhi bit to MIR
the isPHI() function relied on the MIR opcode. This was fine as AFAIK no real target has a real PHI instruction. With SPIR-V, this assumption breaks. The MachineVerifier has a special handling for PHI instructions to check liveness, but since this relied on the PHI/G_PHI opcode check, it was raising an error when the OpPhi MIR was checked. Since the SPIR-V opcode is specific to the backend, I don't think checking the actual opcode in the MachineVerifier code is correct, so I added a bit in the instruction description, and applied it to the 3 existing PHI instruction I found (G_PHI, PHI, OpPhi). Another different bit is the index of the first BB/reg pair: %res = PHI [reg, BB] %res = OpPhi %reg [reg, BB] The solution I had is to have a function in the MachineInstr returning the start index, allowing the MachineVerifier to work on both formats. Its slightly better, it works, but it's not THAT great. An alternative could be to add the index in the MCInstrDesc, this way, the index bit definition would be in the TD files, closer to the definition. This patch reduces the amount of failling tests with EXPENSIVE_CHECKS from 120 to 113 (All fixed are in the SPIR-V backend). Fixes llvm#108844 Signed-off-by: Nathan Gauër <[email protected]>
1 parent f43ad88 commit 7334a28

File tree

9 files changed

+32
-7
lines changed

9 files changed

+32
-7
lines changed

llvm/include/llvm/CodeGen/MachineInstr.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,10 +1394,19 @@ class MachineInstr
13941394
return getOpcode() == TargetOpcode::JUMP_TABLE_DEBUG_INFO;
13951395
}
13961396

1397-
bool isPHI() const {
1398-
return getOpcode() == TargetOpcode::PHI ||
1399-
getOpcode() == TargetOpcode::G_PHI;
1397+
bool isPHI() const { return getDesc().isPhi(); }
1398+
1399+
unsigned getIndexFirstPHIPair() const {
1400+
assert(isPHI());
1401+
1402+
if (getOpcode() == TargetOpcode::G_PHI || getOpcode() == TargetOpcode::PHI)
1403+
return 1;
1404+
// The only other instruction marked as PHI node is OpPhi, in the SPIR-V
1405+
// backend. The only difference is the [reg, BB] pairs starts at index 2,
1406+
// not 1.
1407+
return 2;
14001408
}
1409+
14011410
bool isKill() const { return getOpcode() == TargetOpcode::KILL; }
14021411
bool isImplicitDef() const { return getOpcode()==TargetOpcode::IMPLICIT_DEF; }
14031412
bool isInlineAsm() const {

llvm/include/llvm/MC/MCInstrDesc.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ enum Flag {
187187
Trap,
188188
VariadicOpsAreDefs,
189189
Authenticated,
190+
Phi,
190191
};
191192
} // namespace MCID
192193

@@ -292,6 +293,10 @@ class MCInstrDesc {
292293
/// unconditional branches and return instructions.
293294
bool isBarrier() const { return Flags & (1ULL << MCID::Barrier); }
294295

296+
/// Returns true if this instruction acts as a PHI node.
297+
/// Not all PHI operands need to dominates the definition.
298+
bool isPhi() const { return Flags & (1ULL << MCID::Phi); }
299+
295300
/// Returns true if this instruction part of the terminator for
296301
/// a basic block. Typically this is things like return and branch
297302
/// instructions.

llvm/include/llvm/Target/GenericOpcodes.td

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ def G_PHI : GenericInstruction {
9696
let OutOperandList = (outs type0:$dst);
9797
let InOperandList = (ins variable_ops);
9898
let hasSideEffects = false;
99+
let isPhi = true;
99100
}
100101

101102
def G_FRAME_INDEX : GenericInstruction {

llvm/include/llvm/Target/Target.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,7 @@ class Instruction : InstructionEncoding {
635635
bit isBitcast = false; // Is this instruction a bitcast instruction?
636636
bit isSelect = false; // Is this instruction a select instruction?
637637
bit isBarrier = false; // Can control flow fall through this instruction?
638+
bit isPhi = false; // Is this instruction a phi instruction?
638639
bit isCall = false; // Is this instruction a call instruction?
639640
bit isAdd = false; // Is this instruction an add instruction?
640641
bit isTrap = false; // Is this instruction a trap instruction?
@@ -1174,6 +1175,7 @@ def PHI : StandardPseudoInstruction {
11741175
let InOperandList = (ins variable_ops);
11751176
let AsmString = "PHINODE";
11761177
let hasSideEffects = false;
1178+
let isPhi = true;
11771179
}
11781180
def INLINEASM : StandardPseudoInstruction {
11791181
let OutOperandList = (outs);

llvm/lib/CodeGen/MachineVerifier.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3213,7 +3213,8 @@ void MachineVerifier::calcRegsRequired() {
32133213

32143214
// Handle the PHI node.
32153215
for (const MachineInstr &MI : MBB.phis()) {
3216-
for (unsigned i = 1, e = MI.getNumOperands(); i != e; i += 2) {
3216+
for (unsigned i = MI.getIndexFirstPHIPair(), e = MI.getNumOperands();
3217+
i != e; i += 2) {
32173218
// Skip those Operands which are undef regs or not regs.
32183219
if (!MI.getOperand(i).isReg() || !MI.getOperand(i).readsReg())
32193220
continue;
@@ -3268,7 +3269,8 @@ void MachineVerifier::checkPHIOps(const MachineBasicBlock &MBB) {
32683269
if (!DefReg.isVirtual())
32693270
report("Expected first PHI operand to be a virtual register", &MODef, 0);
32703271

3271-
for (unsigned I = 1, E = Phi.getNumOperands(); I != E; I += 2) {
3272+
for (unsigned I = Phi.getIndexFirstPHIPair(), E = Phi.getNumOperands();
3273+
I != E; I += 2) {
32723274
const MachineOperand &MO0 = Phi.getOperand(I);
32733275
if (!MO0.isReg()) {
32743276
report("Expected PHI operand to be a register", &MO0, I);

llvm/lib/Target/SPIRV/SPIRVInstrInfo.td

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -615,8 +615,10 @@ def OpFwidthCoarse: UnOp<"OpFwidthCoarse", 215>;
615615

616616
// 3.42.17 Control-Flow Instructions
617617

618-
def OpPhi: Op<245, (outs ID:$res), (ins TYPE:$type, ID:$var0, ID:$block0, variable_ops),
619-
"$res = OpPhi $type $var0 $block0">;
618+
let isPhi = 1 in {
619+
def OpPhi: Op<245, (outs ID:$res), (ins TYPE:$type, ID:$var0, ID:$block0, variable_ops),
620+
"$res = OpPhi $type $var0 $block0">;
621+
}
620622
def OpLoopMerge: Op<246, (outs), (ins unknown:$merge, unknown:$continue, LoopControl:$lc, variable_ops),
621623
"OpLoopMerge $merge $continue $lc">;
622624
def OpSelectionMerge: Op<247, (outs), (ins unknown:$merge, SelectionControl:$sc),

llvm/utils/TableGen/Common/CodeGenInstruction.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ CodeGenInstruction::CodeGenInstruction(const Record *R)
469469
FastISelShouldIgnore = R->getValueAsBit("FastISelShouldIgnore");
470470
variadicOpsAreDefs = R->getValueAsBit("variadicOpsAreDefs");
471471
isAuthenticated = R->getValueAsBit("isAuthenticated");
472+
isPhi = R->getValueAsBit("isPhi");
472473

473474
bool Unset;
474475
mayLoad = R->getValueAsBitOrUnset("mayLoad", Unset);

llvm/utils/TableGen/Common/CodeGenInstruction.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ class CodeGenInstruction {
286286
bool hasChain_Inferred : 1;
287287
bool variadicOpsAreDefs : 1;
288288
bool isAuthenticated : 1;
289+
bool isPhi : 1;
289290

290291
std::string DeprecatedReason;
291292
bool HasComplexDeprecationPredicate;

llvm/utils/TableGen/InstrInfoEmitter.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,6 +1240,8 @@ void InstrInfoEmitter::emitRecord(
12401240
OS << "|(1ULL<<MCID::Select)";
12411241
if (Inst.isBarrier)
12421242
OS << "|(1ULL<<MCID::Barrier)";
1243+
if (Inst.isPhi)
1244+
OS << "|(1ULL<<MCID::Phi)";
12431245
if (Inst.hasDelaySlot)
12441246
OS << "|(1ULL<<MCID::DelaySlot)";
12451247
if (Inst.isCall)

0 commit comments

Comments
 (0)