From 2a901401fe453ca5b17048f7e6d74d9c8c91cbf9 Mon Sep 17 00:00:00 2001 From: Puyan Lotfi Date: Fri, 31 May 2019 04:49:58 +0000 Subject: [PATCH] [MIR-Canon] Hardening propagateLocalCopies. This is am almost NFC, it does the following: - If there is no register class for a COPY's src or dst, bail. - Fixes uses iterator invalidation bug. Differential Revision: https://reviews.llvm.org/D62713 llvm-svn: 362191 --- llvm/lib/CodeGen/MIRCanonicalizerPass.cpp | 16 ++++++++++++---- llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir | 9 +++++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/llvm/lib/CodeGen/MIRCanonicalizerPass.cpp b/llvm/lib/CodeGen/MIRCanonicalizerPass.cpp index 650240e..d36c0c8 100644 --- a/llvm/lib/CodeGen/MIRCanonicalizerPass.cpp +++ b/llvm/lib/CodeGen/MIRCanonicalizerPass.cpp @@ -343,15 +343,23 @@ static bool propagateLocalCopies(MachineBasicBlock *MBB) { continue; if (!TargetRegisterInfo::isVirtualRegister(Src)) continue; + // Not folding COPY instructions if regbankselect has not set the RCs. + // Why are we only considering Register Classes? Because the verifier + // sometimes gets upset if the register classes don't match even if the + // types do. A future patch might add COPY folding for matching types in + // pre-registerbankselect code. + if (!MRI.getRegClassOrNull(Dst)) + continue; if (MRI.getRegClass(Dst) != MRI.getRegClass(Src)) continue; - for (auto UI = MRI.use_begin(Dst); UI != MRI.use_end(); ++UI) { - MachineOperand *MO = &*UI; + std::vector Uses; + for (auto UI = MRI.use_begin(Dst); UI != MRI.use_end(); ++UI) + Uses.push_back(&*UI); + for (auto *MO : Uses) MO->setReg(Src); - Changed = true; - } + Changed = true; MI->eraseFromParent(); } diff --git a/llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir b/llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir index 65ad2ff..005014d 100644 --- a/llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir +++ b/llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir @@ -1,8 +1,13 @@ # RUN: llc -o - -march=amdgcn -run-pass mir-canonicalizer -x mir %s | FileCheck %s +# CHECK: %namedVReg4354:vgpr_32 = COPY $vgpr0 # CHECK: %namedVReg1352:vgpr_32 = COPY %namedVReg4353 -# CHECK: %namedVReg1359:vgpr_32 = COPY %namedVReg1362 -# CHECK: %namedVReg1360:vgpr_32 = COPY %namedVReg1363 +# CHECK-NEXT: %namedVReg1358:vgpr_32 = COPY %namedVReg1361 +# CHECK-NEXT: %namedVReg1359:vgpr_32 = COPY %namedVReg1362 +# CHECK-NEXT: %namedVReg1353:vreg_64 = REG_SEQUENCE %namedVReg4354, %subreg.sub0, %namedVReg1352, %subreg.sub1 +# CHECK-NEXT: %namedVReg1354:sgpr_128 = REG_SEQUENCE %namedVReg4354, %subreg.sub0, %namedVReg1352, %subreg.sub1, %namedVReg1358, %subreg.sub2, %namedVReg1359, %subreg.sub3 +# This tests for the itereator invalidation fix (reviews.llvm.org/D62713) +# CHECK-NEXT: BUFFER_STORE_DWORD_ADDR64 %namedVReg1352, %namedVReg1353, %namedVReg1354, 0, 0, 0, 0, 0, 0, implicit $exec ... --- name: foo -- 2.7.4