From c5df1fd8db52905a5af2cb02716e4d3300ef2a72 Mon Sep 17 00:00:00 2001 From: Jonas Paulsson Date: Thu, 31 Oct 2024 14:48:31 +0100 Subject: [PATCH 1/6] Allow KILL MI with dangling MMO in MachineVerifier. --- llvm/lib/CodeGen/MachineVerifier.cpp | 2 +- .../CodeGen/SystemZ/machine-ver-kill-memop.ll | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index e2c09fe25d55c..cc5acccdc8df6 100644 --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -2271,7 +2271,7 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) { // Check the MachineMemOperands for basic consistency. for (MachineMemOperand *Op : MI->memoperands()) { - if (Op->isLoad() && !MI->mayLoad()) + if (Op->isLoad() && (!MI->mayLoad() && !MI->isKill())) report("Missing mayLoad flag", MI); if (Op->isStore() && !MI->mayStore()) report("Missing mayStore flag", MI); diff --git a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll new file mode 100644 index 0000000000000..b3cbc332f473a --- /dev/null +++ b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll @@ -0,0 +1,30 @@ +; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z16 -disable-machine-dce \ +; RUN: -verify-machineinstrs -O3 +; +; Test that this passes the verifier even though a KILL instruction with a +; dangling memory operand emerges. + +define void @fun(ptr %Src, ptr %Dst) { + br label %2 + +2: + %3 = load i32, ptr %Src, align 4 + %4 = freeze i32 %3 + %5 = icmp eq i32 %4, 0 + %6 = load i32, ptr poison, align 4 + %7 = select i1 %5, i32 0, i32 %6 + %8 = load i32, ptr %Src, align 4 + %9 = freeze i32 %8 + %10 = icmp eq i32 %9, 0 + %11 = load i32, ptr poison, align 4 + %12 = select i1 %10, i32 0, i32 %11 + br label %13 + +13: + %14 = phi i32 [ %12, %13 ], [ %7, %2 ] + %15 = icmp slt i32 %14, 5 + %16 = zext i1 %15 to i64 + %17 = xor i64 poison, %16 + store i64 %17, ptr %Dst, align 8 + br label %13 +} From f03ba98cae95ae64800ffc8d4d1de555375504b3 Mon Sep 17 00:00:00 2001 From: Jonas Paulsson Date: Mon, 4 Nov 2024 15:19:42 -0600 Subject: [PATCH 2/6] Remove the MMO instead in LiveRangeEdit. --- llvm/lib/CodeGen/LiveRangeEdit.cpp | 1 + llvm/lib/CodeGen/MachineVerifier.cpp | 2 +- llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/llvm/lib/CodeGen/LiveRangeEdit.cpp b/llvm/lib/CodeGen/LiveRangeEdit.cpp index c3c581d42236e..7b630e88b2a60 100644 --- a/llvm/lib/CodeGen/LiveRangeEdit.cpp +++ b/llvm/lib/CodeGen/LiveRangeEdit.cpp @@ -385,6 +385,7 @@ void LiveRangeEdit::eliminateDeadDef(MachineInstr *MI, ToShrinkSet &ToShrink) { continue; MI->removeOperand(i-1); } + MI->dropMemRefs(*MI->getMF()); LLVM_DEBUG(dbgs() << "Converted physregs to:\t" << *MI); } else { // If the dest of MI is an original reg and MI is reMaterializable, diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index cc5acccdc8df6..e2c09fe25d55c 100644 --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -2271,7 +2271,7 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) { // Check the MachineMemOperands for basic consistency. for (MachineMemOperand *Op : MI->memoperands()) { - if (Op->isLoad() && (!MI->mayLoad() && !MI->isKill())) + if (Op->isLoad() && !MI->mayLoad()) report("Missing mayLoad flag", MI); if (Op->isStore() && !MI->mayStore()) report("Missing mayStore flag", MI); diff --git a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll index b3cbc332f473a..793fd3908b9f8 100644 --- a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll +++ b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll @@ -1,8 +1,8 @@ ; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z16 -disable-machine-dce \ ; RUN: -verify-machineinstrs -O3 ; -; Test that this passes the verifier even though a KILL instruction with a -; dangling memory operand emerges. +; Test that the MemoryOperand of the produced KILL instruction is removed +; and as a result the machine verifier succeeds. define void @fun(ptr %Src, ptr %Dst) { br label %2 From 772910b77b2195829a6827e2d32807f1b996ff98 Mon Sep 17 00:00:00 2001 From: Jonas Paulsson Date: Mon, 4 Nov 2024 15:40:49 -0600 Subject: [PATCH 3/6] Make a simpler .mir test. --- .../CodeGen/SystemZ/machine-ver-kill-memop.ll | 30 ----------- .../SystemZ/machine-ver-kill-memop.mir | 52 +++++++++++++++++++ 2 files changed, 52 insertions(+), 30 deletions(-) delete mode 100644 llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll create mode 100644 llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir diff --git a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll deleted file mode 100644 index 793fd3908b9f8..0000000000000 --- a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.ll +++ /dev/null @@ -1,30 +0,0 @@ -; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z16 -disable-machine-dce \ -; RUN: -verify-machineinstrs -O3 -; -; Test that the MemoryOperand of the produced KILL instruction is removed -; and as a result the machine verifier succeeds. - -define void @fun(ptr %Src, ptr %Dst) { - br label %2 - -2: - %3 = load i32, ptr %Src, align 4 - %4 = freeze i32 %3 - %5 = icmp eq i32 %4, 0 - %6 = load i32, ptr poison, align 4 - %7 = select i1 %5, i32 0, i32 %6 - %8 = load i32, ptr %Src, align 4 - %9 = freeze i32 %8 - %10 = icmp eq i32 %9, 0 - %11 = load i32, ptr poison, align 4 - %12 = select i1 %10, i32 0, i32 %11 - br label %13 - -13: - %14 = phi i32 [ %12, %13 ], [ %7, %2 ] - %15 = icmp slt i32 %14, 5 - %16 = zext i1 %15 to i64 - %17 = xor i64 poison, %16 - store i64 %17, ptr %Dst, align 8 - br label %13 -} diff --git a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir new file mode 100644 index 0000000000000..d4eb7ff1a7040 --- /dev/null +++ b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir @@ -0,0 +1,52 @@ +# RUN: llc -o /dev/null %s -mtriple=s390x-linux-gnu -mcpu=z16 \ +# RUN: -disable-machine-dce -verify-machineinstrs -run-pass=register-coalescer + +# Test that the MemoryOperand of the produced KILL instruction is removed +# and as a result the machine verifier succeeds. + +--- | + define void @fun(ptr %Src, ptr %Dst, ptr %Src2) { + ret void + } +... +--- +name: fun +alignment: 16 +tracksRegLiveness: true +noPhis: true +isSSA: false +noVRegs: false +hasFakeUses: false +registers: + - { id: 0, class: grx32bit } + - { id: 1, class: grx32bit } + - { id: 2, class: grx32bit } + - { id: 3, class: addr64bit } + - { id: 4, class: gr64bit } + - { id: 5, class: grx32bit } + - { id: 6, class: grx32bit } + - { id: 7, class: grx32bit } + - { id: 8, class: addr64bit } +liveins: + - { reg: '$r2d', virtual-reg: '%3' } +frameInfo: + maxAlignment: 1 +machineFunctionInfo: {} +body: | + bb.0: + liveins: $r2d + + %3:addr64bit = COPY killed $r2d + + bb.1: + %5:grx32bit = LMux killed %3, 0, $noreg :: (load (s32) from %ir.Src) + CHIMux killed %5, 0, implicit-def $cc + %7:grx32bit = LHIMux 0 + %1:grx32bit = COPY killed %7 + %1:grx32bit = LOCMux %1, undef %8:addr64bit, 0, 14, 6, implicit killed $cc :: (load (s32) from %ir.Src2) + dead %0:grx32bit = COPY killed %1 + + bb.2: + J %bb.2 + +... From f71dc9329840444e2feaf482f79289a91de12a4f Mon Sep 17 00:00:00 2001 From: Jonas Paulsson Date: Mon, 4 Nov 2024 15:47:52 -0600 Subject: [PATCH 4/6] Remove stray llc option in test. --- llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir index d4eb7ff1a7040..71751a9e245c3 100644 --- a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir +++ b/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir @@ -1,5 +1,5 @@ # RUN: llc -o /dev/null %s -mtriple=s390x-linux-gnu -mcpu=z16 \ -# RUN: -disable-machine-dce -verify-machineinstrs -run-pass=register-coalescer +# RUN: -verify-machineinstrs -run-pass=register-coalescer # Test that the MemoryOperand of the produced KILL instruction is removed # and as a result the machine verifier succeeds. From 1ee2226b617119c723f491d40824160be2505212 Mon Sep 17 00:00:00 2001 From: Jonas Paulsson Date: Mon, 4 Nov 2024 16:33:55 -0600 Subject: [PATCH 5/6] Updates to .mir test per review. --- ...chine-ver-kill-memop.mir => kill-remove-memop.mir} | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) rename llvm/test/CodeGen/SystemZ/{machine-ver-kill-memop.mir => kill-remove-memop.mir} (81%) diff --git a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir b/llvm/test/CodeGen/SystemZ/kill-remove-memop.mir similarity index 81% rename from llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir rename to llvm/test/CodeGen/SystemZ/kill-remove-memop.mir index 71751a9e245c3..c432dc1cabbcf 100644 --- a/llvm/test/CodeGen/SystemZ/machine-ver-kill-memop.mir +++ b/llvm/test/CodeGen/SystemZ/kill-remove-memop.mir @@ -2,13 +2,8 @@ # RUN: -verify-machineinstrs -run-pass=register-coalescer # Test that the MemoryOperand of the produced KILL instruction is removed -# and as a result the machine verifier succeeds. +# (and as a result the machine verifier succeeds). ---- | - define void @fun(ptr %Src, ptr %Dst, ptr %Src2) { - ret void - } -... --- name: fun alignment: 16 @@ -39,11 +34,11 @@ body: | %3:addr64bit = COPY killed $r2d bb.1: - %5:grx32bit = LMux killed %3, 0, $noreg :: (load (s32) from %ir.Src) + %5:grx32bit = LMux killed %3, 0, $noreg :: (load (s32)) CHIMux killed %5, 0, implicit-def $cc %7:grx32bit = LHIMux 0 %1:grx32bit = COPY killed %7 - %1:grx32bit = LOCMux %1, undef %8:addr64bit, 0, 14, 6, implicit killed $cc :: (load (s32) from %ir.Src2) + %1:grx32bit = LOCMux %1, undef %8:addr64bit, 0, 14, 6, implicit killed $cc :: (load (s32)) dead %0:grx32bit = COPY killed %1 bb.2: From 48ecf405b0ca2d6580acd7526f7d242840d26031 Mon Sep 17 00:00:00 2001 From: Jonas Paulsson Date: Mon, 4 Nov 2024 21:25:24 -0600 Subject: [PATCH 6/6] Test updated again. --- .../CodeGen/SystemZ/kill-remove-memop.mir | 47 ------------------- .../SystemZ/liverangeedit-kill-memop.mir | 29 ++++++++++++ 2 files changed, 29 insertions(+), 47 deletions(-) delete mode 100644 llvm/test/CodeGen/SystemZ/kill-remove-memop.mir create mode 100644 llvm/test/CodeGen/SystemZ/liverangeedit-kill-memop.mir diff --git a/llvm/test/CodeGen/SystemZ/kill-remove-memop.mir b/llvm/test/CodeGen/SystemZ/kill-remove-memop.mir deleted file mode 100644 index c432dc1cabbcf..0000000000000 --- a/llvm/test/CodeGen/SystemZ/kill-remove-memop.mir +++ /dev/null @@ -1,47 +0,0 @@ -# RUN: llc -o /dev/null %s -mtriple=s390x-linux-gnu -mcpu=z16 \ -# RUN: -verify-machineinstrs -run-pass=register-coalescer - -# Test that the MemoryOperand of the produced KILL instruction is removed -# (and as a result the machine verifier succeeds). - ---- -name: fun -alignment: 16 -tracksRegLiveness: true -noPhis: true -isSSA: false -noVRegs: false -hasFakeUses: false -registers: - - { id: 0, class: grx32bit } - - { id: 1, class: grx32bit } - - { id: 2, class: grx32bit } - - { id: 3, class: addr64bit } - - { id: 4, class: gr64bit } - - { id: 5, class: grx32bit } - - { id: 6, class: grx32bit } - - { id: 7, class: grx32bit } - - { id: 8, class: addr64bit } -liveins: - - { reg: '$r2d', virtual-reg: '%3' } -frameInfo: - maxAlignment: 1 -machineFunctionInfo: {} -body: | - bb.0: - liveins: $r2d - - %3:addr64bit = COPY killed $r2d - - bb.1: - %5:grx32bit = LMux killed %3, 0, $noreg :: (load (s32)) - CHIMux killed %5, 0, implicit-def $cc - %7:grx32bit = LHIMux 0 - %1:grx32bit = COPY killed %7 - %1:grx32bit = LOCMux %1, undef %8:addr64bit, 0, 14, 6, implicit killed $cc :: (load (s32)) - dead %0:grx32bit = COPY killed %1 - - bb.2: - J %bb.2 - -... diff --git a/llvm/test/CodeGen/SystemZ/liverangeedit-kill-memop.mir b/llvm/test/CodeGen/SystemZ/liverangeedit-kill-memop.mir new file mode 100644 index 0000000000000..8cfa22d2469e2 --- /dev/null +++ b/llvm/test/CodeGen/SystemZ/liverangeedit-kill-memop.mir @@ -0,0 +1,29 @@ +# RUN: llc -o /dev/null %s -mtriple=s390x-linux-gnu -mcpu=z16 \ +# RUN: -verify-machineinstrs -run-pass=register-coalescer + +# The LOCMux below produces a dead definition and will be turned into +# a KILL instruction (by LiveRangeEdit::eliminateDeadDef()). When this +# happens, the MemoryOperand must also be removed as this is required +# by the machine verifier. + +--- +name: fun +tracksRegLiveness: true +body: | + bb.0: + liveins: $r2d + + %3:addr64bit = COPY killed $r2d + + bb.1: + %5:grx32bit = LMux killed %3, 0, $noreg :: (load (s32)) + CHIMux killed %5, 0, implicit-def $cc + %7:grx32bit = LHIMux 0 + %1:grx32bit = COPY killed %7 + %1:grx32bit = LOCMux %1, undef %8:addr64bit, 0, 14, 6, implicit killed $cc :: (load (s32)) + dead %0:grx32bit = COPY killed %1 + + bb.2: + J %bb.2 + +...