[llvm] [MachineVerifier] Query TargetInstrInfo for PHI nodes. (PR #110507)

Nathan Gauër via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 30 08:33:18 PDT 2024


================
@@ -2278,6 +2278,32 @@ class TargetInstrInfo : public MCInstrInfo {
     llvm_unreachable("unknown number of operands necessary");
   }
 
+  // This this instruction a PHI node.
+  // This function should be favored over MI.isPHI() as it allows backends to
+  // define additional PHI instructions.
+  virtual bool isPhiInstr(const MachineInstr &MI) const {
+    return MI.getOpcode() == TargetOpcode::G_PHI ||
+           MI.getOpcode() == TargetOpcode::PHI;
+  }
+
+  // Returns the number of [Value, Src] pairs this phi instruction has.
+  // Only valid to call on a phi node.
+  virtual unsigned getNumPhiIncomingPair(const MachineInstr &MI) const {
+    assert(isPhiInstr(MI));
+    // G_PHI/PHI only have a single operand before the pairs. 2 Operands per
+    // pair.
+    return (MI.getNumOperands() - 1) / 2;
+  }
+
+  // Returns the |index|'th [Value, Src] pair of this phi instruction.
+  virtual std::pair<MachineOperand, MachineBasicBlock *>
+  getPhiIncomingPair(const MachineInstr &MI, unsigned index) const {
+    // Behavior for G_PHI/PHI instructions.
+    MachineOperand Operand = MI.getOperand(index * 2 + 1);
+    MachineBasicBlock *MBB = MI.getOperand(index * 2 + 2).getMBB();
+    return {Operand, MBB};
+  }
----------------
Keenuts wrote:

Sure.
A failing test which now passes is `llvm/test/CodeGen/SPIRV/branching/if-merging.ll`.
Added `-verify-machineinstrs` to fixed tests to the current PR (adding this flag on main break those)

```
bb.1.entry:
  successors: %bb.2, %bb.3

  %20:type = OpTypeBool
  %2:type = OpTypeInt 32, 0
  OpName %0:iid, 97
  OpName %1:iid, 98
  %4:type = OpTypeFunction %2:type, %2:type, %2:type
  %3:iid = OpFunction %2:type, 0, %4:type
  %0:iid = OpFunctionParameter %2:type
  %1:iid = OpFunctionParameter %2:type
  OpName %3:iid, 1953719668, 6711647
  OpDecorate %3:iid, 41, 1953719668, 6711647, 0
  %10:type = OpTypeFunction %2:type
  %9:iid = OpFunction %2:type, 0, %10:type
  OpName %9:iid, 7496034
  OpDecorate %9:iid, 41, 7496034, 1
  %13:iid = OpFunction %2:type, 0, %10:type
  OpName %13:iid, 7303014
  OpDecorate %13:iid, 41, 7303014, 1
  %5:iid = OpIEqual %20:type, %0:iid, %1:iid
  OpName %5:iid, 1684959075, 0
  OpBranchConditional %5:iid, %bb.2, %bb.3

bb.2.true_label:
; predecessors: %bb.1
  successors: %bb.4(0x80000000); %bb.4(100.00%)

  %12:iid = OpFunctionCall %2:type, @foo
  OpName %12:iid, 12662
  OpBranch %bb.4

bb.3.false_label:
; predecessors: %bb.1
  successors: %bb.4(0x80000000); %bb.4(100.00%)

  %8:iid = OpFunctionCall %2:type, @bar
  OpName %8:iid, 12918
  OpBranch %bb.4

bb.4.merge_label:
; predecessors: %bb.3, %bb.2

  %15:id = OpPhi %2:type, %12:iid, %bb.2, %8:iid, %bb.3
  OpName %15:id, 118
  OpReturnValue %15:id

# End machine code for function test_if.

*** Bad machine code: Virtual register defs don't dominate all uses. ***
- function:    test_if
- v. register: %8

*** Bad machine code: Virtual register defs don't dominate all uses. ***
- function:    test_if
- v. register: %12
```

Is the `OpPhi` was handled as a PHI, it would pass MachineVerifer tests, as `%12` is defined if coming from `bb.2`, and same for `%8/bb.3`.


https://github.com/llvm/llvm-project/pull/110507


More information about the llvm-commits mailing list