From 94ebd7d9ff1776bbc94ca6ac82a783fa9d4eaa72 Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Wed, 21 Sep 2022 10:42:03 -0400 Subject: [PATCH] MachineVerifier: Verify REG_SEQUENCE Somehow there was no verification of this, other than an ad-hoc assertion in TwoAddressInstructions. --- llvm/lib/CodeGen/MachineOperand.cpp | 2 +- llvm/lib/CodeGen/MachineVerifier.cpp | 30 +++++++++++ llvm/lib/CodeGen/TwoAddressInstructionPass.cpp | 5 -- llvm/test/CodeGen/AMDGPU/load-store-opt-dlc.mir | 8 +-- llvm/test/CodeGen/AMDGPU/load-store-opt-scc.mir | 8 +-- .../CodeGen/MIR/X86/subregister-index-operands.mir | 13 +++-- llvm/test/MachineVerifier/verify-reg-sequence.mir | 59 ++++++++++++++++++++++ 7 files changed, 106 insertions(+), 19 deletions(-) create mode 100644 llvm/test/MachineVerifier/verify-reg-sequence.mir diff --git a/llvm/lib/CodeGen/MachineOperand.cpp b/llvm/lib/CodeGen/MachineOperand.cpp index df52741..797b3de 100644 --- a/llvm/lib/CodeGen/MachineOperand.cpp +++ b/llvm/lib/CodeGen/MachineOperand.cpp @@ -531,7 +531,7 @@ static void printFrameIndex(raw_ostream& OS, int FrameIndex, bool IsFixed, void MachineOperand::printSubRegIdx(raw_ostream &OS, uint64_t Index, const TargetRegisterInfo *TRI) { OS << "%subreg."; - if (TRI) + if (TRI && Index != 0 && Index < TRI->getNumSubRegIndices()) OS << TRI->getSubRegIndexName(Index); else OS << Index; diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index a5aef3b..3cb2049 100644 --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -1923,6 +1923,36 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) { break; } } break; + case TargetOpcode::REG_SEQUENCE: { + unsigned NumOps = MI->getNumOperands(); + if (!(NumOps & 1)) { + report("Invalid number of operands for REG_SEQUENCE", MI); + break; + } + + for (unsigned I = 1; I != NumOps; I += 2) { + const MachineOperand &RegOp = MI->getOperand(I); + const MachineOperand &SubRegOp = MI->getOperand(I + 1); + + if (!RegOp.isReg()) + report("Invalid register operand for REG_SEQUENCE", &RegOp, I); + + if (!SubRegOp.isImm() || SubRegOp.getImm() == 0 || + SubRegOp.getImm() >= TRI->getNumSubRegIndices()) { + report("Invalid subregister index operand for REG_SEQUENCE", + &SubRegOp, I + 1); + } + } + + Register DstReg = MI->getOperand(0).getReg(); + if (DstReg.isPhysical()) + report("REG_SEQUENCE does not support physical register results", MI); + + if (MI->getOperand(0).getSubReg()) + report("Invalid subreg result for REG_SEQUENCE", MI); + + break; + } } } diff --git a/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp b/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp index 3805221..a487ec3 100644 --- a/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp +++ b/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp @@ -1887,11 +1887,6 @@ void TwoAddressInstructionPass:: eliminateRegSequence(MachineBasicBlock::iterator &MBBI) { MachineInstr &MI = *MBBI; Register DstReg = MI.getOperand(0).getReg(); - if (MI.getOperand(0).getSubReg() || DstReg.isPhysical() || - !(MI.getNumOperands() & 1)) { - LLVM_DEBUG(dbgs() << "Illegal REG_SEQUENCE instruction:" << MI); - llvm_unreachable(nullptr); - } SmallVector OrigRegs; if (LIS) { diff --git a/llvm/test/CodeGen/AMDGPU/load-store-opt-dlc.mir b/llvm/test/CodeGen/AMDGPU/load-store-opt-dlc.mir index 56aec7c..0cca309 100644 --- a/llvm/test/CodeGen/AMDGPU/load-store-opt-dlc.mir +++ b/llvm/test/CodeGen/AMDGPU/load-store-opt-dlc.mir @@ -51,7 +51,7 @@ body: | %1:sgpr_64 = S_LOAD_DWORDX2_IMM %1, 36, 0 :: (dereferenceable invariant load (s64) from `i64 addrspace(4)* undef`, addrspace 4) %2:sgpr_32 = COPY $sgpr2 %3:sgpr_32 = COPY $sgpr3 - %4:sgpr_128 = REG_SEQUENCE %1, %2, %3 + %4:sgpr_128 = REG_SEQUENCE %1, %subreg.sub0, %2, %subreg.sub1, %3, %subreg.sub2 %5:vgpr_32 = COPY $vgpr0 %6:vgpr_32 = COPY $vgpr1 @@ -82,7 +82,7 @@ body: | %1:sgpr_64 = S_LOAD_DWORDX2_IMM %1, 36, 0 :: (dereferenceable invariant load (s64) from `i64 addrspace(4)* undef`, addrspace 4) %2:sgpr_32 = COPY $sgpr2 %3:sgpr_32 = COPY $sgpr3 - %4:sgpr_128 = REG_SEQUENCE %1, %2, %3 + %4:sgpr_128 = REG_SEQUENCE %1, %subreg.sub0, %2, %subreg.sub1, %3, %subreg.sub2 %5:vgpr_32 = COPY $vgpr0 %6:vgpr_32 = COPY $vgpr1 @@ -113,7 +113,7 @@ body: | %1:sgpr_64 = S_LOAD_DWORDX2_IMM %1, 36, 0 :: (dereferenceable invariant load (s64) from `i64 addrspace(4)* undef`, addrspace 4) %2:sgpr_32 = COPY $sgpr2 %3:sgpr_32 = COPY $sgpr3 - %4:sgpr_128 = REG_SEQUENCE %1, %2, %3 + %4:sgpr_128 = REG_SEQUENCE %1, %subreg.sub0, %2, %subreg.sub1, %3, %subreg.sub2 %5:vgpr_32 = COPY $vgpr0 %6:vgpr_32 = COPY $vgpr1 @@ -143,7 +143,7 @@ body: | %1:sgpr_64 = S_LOAD_DWORDX2_IMM %1, 36, 0 :: (dereferenceable invariant load (s64) from `i64 addrspace(4)* undef`, addrspace 4) %2:sgpr_32 = COPY $sgpr2 %3:sgpr_32 = COPY $sgpr3 - %4:sgpr_128 = REG_SEQUENCE %1, %2, %3 + %4:sgpr_128 = REG_SEQUENCE %1, %subreg.sub0, %2, %subreg.sub1, %3, %subreg.sub2 %5:vgpr_32 = COPY $vgpr0 %6:vgpr_32 = COPY $vgpr1 diff --git a/llvm/test/CodeGen/AMDGPU/load-store-opt-scc.mir b/llvm/test/CodeGen/AMDGPU/load-store-opt-scc.mir index 2d3c101..baaeb4e 100644 --- a/llvm/test/CodeGen/AMDGPU/load-store-opt-scc.mir +++ b/llvm/test/CodeGen/AMDGPU/load-store-opt-scc.mir @@ -51,7 +51,7 @@ body: | %1:sgpr_64 = S_LOAD_DWORDX2_IMM %1, 36, 0 :: (dereferenceable invariant load (s64) from `i64 addrspace(4)* undef`, addrspace 4) %2:sgpr_32 = COPY $sgpr2 %3:sgpr_32 = COPY $sgpr3 - %4:sgpr_128 = REG_SEQUENCE %1, %2, %3 + %4:sgpr_128 = REG_SEQUENCE %1, %subreg.sub0, %2, %subreg.sub1, %3, %subreg.sub2 %5:vgpr_32 = COPY $vgpr0 %6:vgpr_32 = COPY $vgpr1 @@ -82,7 +82,7 @@ body: | %1:sgpr_64 = S_LOAD_DWORDX2_IMM %1, 36, 0 :: (dereferenceable invariant load (s64) from `i64 addrspace(4)* undef`, addrspace 4) %2:sgpr_32 = COPY $sgpr2 %3:sgpr_32 = COPY $sgpr3 - %4:sgpr_128 = REG_SEQUENCE %1, %2, %3 + %4:sgpr_128 = REG_SEQUENCE %1, %subreg.sub0, %2, %subreg.sub1, %3, %subreg.sub2 %5:vgpr_32 = COPY $vgpr0 %6:vgpr_32 = COPY $vgpr1 @@ -113,7 +113,7 @@ body: | %1:sgpr_64 = S_LOAD_DWORDX2_IMM %1, 36, 0 :: (dereferenceable invariant load (s64) from `i64 addrspace(4)* undef`, addrspace 4) %2:sgpr_32 = COPY $sgpr2 %3:sgpr_32 = COPY $sgpr3 - %4:sgpr_128 = REG_SEQUENCE %1, %2, %3 + %4:sgpr_128 = REG_SEQUENCE %1, %subreg.sub0, %2, %subreg.sub1, %3, %subreg.sub2 %5:vgpr_32 = COPY $vgpr0 %6:vgpr_32 = COPY $vgpr1 @@ -143,7 +143,7 @@ body: | %1:sgpr_64 = S_LOAD_DWORDX2_IMM %1, 36, 0 :: (dereferenceable invariant load (s64) from `i64 addrspace(4)* undef`, addrspace 4) %2:sgpr_32 = COPY $sgpr2 %3:sgpr_32 = COPY $sgpr3 - %4:sgpr_128 = REG_SEQUENCE %1, %2, %3 + %4:sgpr_128 = REG_SEQUENCE %1, %subreg.sub0, %2, %subreg.sub1, %3, %subreg.sub2 %5:vgpr_32 = COPY $vgpr0 %6:vgpr_32 = COPY $vgpr1 diff --git a/llvm/test/CodeGen/MIR/X86/subregister-index-operands.mir b/llvm/test/CodeGen/MIR/X86/subregister-index-operands.mir index 383499f..f34816b 100644 --- a/llvm/test/CodeGen/MIR/X86/subregister-index-operands.mir +++ b/llvm/test/CodeGen/MIR/X86/subregister-index-operands.mir @@ -22,13 +22,16 @@ body: | liveins: $edi, $eax ; CHECK-LABEL: name: t ; CHECK: liveins: $edi, $eax - ; CHECK: [[INSERT_SUBREG:%[0-9]+]]:gr32 = INSERT_SUBREG $edi, $al, %subreg.sub_8bit - ; CHECK: [[EXTRACT_SUBREG:%[0-9]+]]:gr8 = EXTRACT_SUBREG $eax, %subreg.sub_8bit_hi - ; CHECK: $ax = REG_SEQUENCE [[EXTRACT_SUBREG]], %subreg.sub_8bit, [[EXTRACT_SUBREG]], %subreg.sub_8bit_hi - ; CHECK: RET64 $ax + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[INSERT_SUBREG:%[0-9]+]]:gr32 = INSERT_SUBREG $edi, $al, %subreg.sub_8bit + ; CHECK-NEXT: [[EXTRACT_SUBREG:%[0-9]+]]:gr8 = EXTRACT_SUBREG $eax, %subreg.sub_8bit_hi + ; CHECK-NEXT: [[REG_SEQUENCE:%[0-9]+]]:gr8 = REG_SEQUENCE [[EXTRACT_SUBREG]], %subreg.sub_8bit, [[EXTRACT_SUBREG]], %subreg.sub_8bit_hi + ; CHECK-NEXT: $ax = COPY [[REG_SEQUENCE]] + ; CHECK-NEXT: RET64 $ax %0 = INSERT_SUBREG $edi, $al, %subreg.sub_8bit %1 = EXTRACT_SUBREG $eax, %subreg.sub_8bit_hi - $ax = REG_SEQUENCE %1, %subreg.sub_8bit, %1, %subreg.sub_8bit_hi + %2:gr8 = REG_SEQUENCE %1, %subreg.sub_8bit, %1, %subreg.sub_8bit_hi + $ax = COPY %2 RET64 $ax ... diff --git a/llvm/test/MachineVerifier/verify-reg-sequence.mir b/llvm/test/MachineVerifier/verify-reg-sequence.mir new file mode 100644 index 0000000..4ce6f70 --- /dev/null +++ b/llvm/test/MachineVerifier/verify-reg-sequence.mir @@ -0,0 +1,59 @@ +# REQUIRES: amdgpu-registered-target +# RUN: not --crash llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -run-pass=none -o /dev/null %s 2>&1 | FileCheck %s + +--- +name: invalid_reg_sequence +tracksRegLiveness: true +body: | + bb.0: + %0:vgpr_32 = IMPLICIT_DEF + %1:vgpr_32 = IMPLICIT_DEF + + ; No operands + ; CHECK: *** Bad machine code: Too few operands *** + REG_SEQUENCE + + ; Only dest operand + ; CHECK: *** Bad machine code: Too few operands *** + %2:vreg_64 = REG_SEQUENCE + + ; Missing destination + ; CHECK: *** Bad machine code: Explicit definition marked as use *** + REG_SEQUENCE %0, %subreg.sub0, %1, %subreg.sub1 + + ; Missing subreg operand + ; CHECK: *** Bad machine code: Invalid number of operands for REG_SEQUENCE *** + %3:vreg_64 = REG_SEQUENCE %0, %subreg.sub0, %1 + + ; Missing register operand + ; CHECK: *** Bad machine code: Invalid number of operands for REG_SEQUENCE *** + %4:vreg_64 = REG_SEQUENCE %0, %subreg.sub0, %subreg.sub1 + + ; Physreg destination + ; CHECK: *** Bad machine code: REG_SEQUENCE does not support physical register results *** + $vgpr0_vgpr1 = REG_SEQUENCE %0, %subreg.sub0, %1, %subreg.sub1 + + ; Subreg in destination + ; CHECK: *** Bad machine code: Invalid subreg result for REG_SEQUENCE *** + %5.sub0_sub1:vreg_128 = REG_SEQUENCE %0, %subreg.sub0, %1, %subreg.sub1 + + ; All operands are registers + ; CHECK: *** Bad machine code: Invalid subregister index operand for REG_SEQUENCE *** + %6:vreg_64 = REG_SEQUENCE %0, %1 + + ; Register and subreg index operand order swapped + ; CHECK: *** Bad machine code: Invalid register operand for REG_SEQUENCE *** + ; CHECK: *** Bad machine code: Invalid subregister index operand for REG_SEQUENCE *** + %7:vreg_64 = REG_SEQUENCE %subreg.sub0, %0, %subreg.sub1, %1 + + ; Invalid subreg index constants + ; CHECK: *** Bad machine code: Invalid subregister index operand for REG_SEQUENCE *** + ; CHECK: - instruction: %8:vreg_64 = REG_SEQUENCE %0:vgpr_32, %subreg.0, %1:vgpr_32, %subreg.99999 + ; CHECK-NEXT: operand 2: 0 + + ; CHECK: *** Bad machine code: Invalid subregister index operand for REG_SEQUENCE *** + ; CHECK: instruction: %8:vreg_64 = REG_SEQUENCE %0:vgpr_32, %subreg.0, %1:vgpr_32, %subreg.99999 + ; CHECK-NEXT: operand 4: 99999 + %8:vreg_64 = REG_SEQUENCE %0, 0, %1, 99999 + +... -- 2.7.4