From: Dmitri Gribenko Date: Mon, 9 Sep 2019 16:46:45 +0000 (+0000) Subject: Revert "[MachineCopyPropagation] Remove redundant copies after TailDup via machine-cp" X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d9c4060bd5c9e6c24a96cd7e4501be30301dad9d;p=platform%2Fupstream%2Fllvm.git Revert "[MachineCopyPropagation] Remove redundant copies after TailDup via machine-cp" This reverts commit 371359. I'm suspecting a miscompile, I posted a reproducer to https://reviews.llvm.org/D65267. llvm-svn: 371421 --- diff --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp index aa9d733..ebe76e3 100644 --- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp +++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp @@ -68,7 +68,6 @@ using namespace llvm; STATISTIC(NumDeletes, "Number of dead copies deleted"); STATISTIC(NumCopyForwards, "Number of copy uses forwarded"); -STATISTIC(NumCopyBackwardPropagated, "Number of copy defs backward propagated"); DEBUG_COUNTER(FwdCounter, "machine-cp-fwd", "Controls which register COPYs are forwarded"); @@ -212,13 +211,11 @@ private: void ReadRegister(unsigned Reg, MachineInstr &Reader, DebugType DT); void CopyPropagateBlock(MachineBasicBlock &MBB); - bool eraseIfRedundant(MachineInstr &Copy); bool eraseIfRedundant(MachineInstr &Copy, unsigned Src, unsigned Def); void forwardUses(MachineInstr &MI); bool isForwardableRegClassCopy(const MachineInstr &Copy, const MachineInstr &UseI, unsigned UseIdx); bool hasImplicitOverlap(const MachineInstr &MI, const MachineOperand &Use); - bool isSafeBackwardCopyPropagation(MachineInstr &Copy, MachineInstr &SrcMI); /// Candidates for deletion. SmallSetVector MaybeDeadCopies; @@ -277,57 +274,6 @@ static bool isNopCopy(const MachineInstr &PreviousCopy, unsigned Src, return SubIdx == TRI->getSubRegIndex(PreviousDef, Def); } -bool MachineCopyPropagation::isSafeBackwardCopyPropagation( - MachineInstr &Copy, MachineInstr &SrcMI) { - MachineOperand &SrcOp = SrcMI.getOperand(0); - if (!(SrcOp.isReg() && SrcOp.isDef() && - SrcOp.getReg() == Copy.getOperand(1).getReg() && SrcOp.isRenamable() && - !SrcOp.isTied() && !SrcOp.isImplicit() && - !MRI->isReserved(SrcOp.getReg()))) - return false; - if (const TargetRegisterClass *URC = SrcMI.getRegClassConstraint(0, TII, TRI)) - return URC->contains(Copy.getOperand(0).getReg()); - return false; -} - -/// In a terminal BB, remove instruction \p Copy if \p Copy's src and dst are -/// not used or defined between \p Copy and definition point of \p Copy's src. -/// \p Copy's dst will be backward propagated to where \p Copy's src is defined. -bool MachineCopyPropagation::eraseIfRedundant(MachineInstr &Copy) { - // Only take terminal BBs into account. - if (!Copy.getParent()->succ_empty()) - return false; - if (!Copy.getOperand(1).isRenamable() || !Copy.getOperand(1).isKill()) - return false; - unsigned Def = Copy.getOperand(0).getReg(); - unsigned Src = Copy.getOperand(1).getReg(); - if (MRI->isReserved(Src) || MRI->isReserved(Def)) - return false; - MachineBasicBlock::reverse_iterator E = Copy.getParent()->rend(), It = Copy; - It++; - MachineInstr *SrcMI = nullptr; - for (; It != E; ++It) { - if (It->readsRegister(Src, TRI) || It->readsRegister(Def, TRI)) - return false; - if (It->modifiesRegister(Def, TRI)) - return false; - if (It->modifiesRegister(Src, TRI)) { - SrcMI = &*It; - break; - } - } - if (!SrcMI) - return false; - if (!isSafeBackwardCopyPropagation(Copy, *SrcMI)) - return false; - SrcMI->getOperand(0).setReg(Def); - SrcMI->getOperand(0).setIsRenamable(Copy.getOperand(0).isRenamable()); - Copy.eraseFromParent(); - ++NumCopyBackwardPropagated; - ++NumDeletes; - return true; -} - /// Remove instruction \p Copy if there exists a previous copy that copies the /// register \p Src to the register \p Def; This may happen indirectly by /// copying the super registers. @@ -529,17 +475,6 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) { !Register::isVirtualRegister(Src) && "MachineCopyPropagation should be run after register allocation!"); - // In a terminal BB, - // $reg0 = OP ... - // ... <<< No uses of $reg0 and $reg1, no defs of $reg0 and $reg1 - // $reg1 = COPY $reg0 <<< $reg0 is killed - // => - // $reg1 = OP ... - // ... - // - if (eraseIfRedundant(*MI)) - continue; - // The two copies cancel out and the source of the first copy // hasn't been overridden, eliminate the second one. e.g. // %ecx = COPY %eax diff --git a/llvm/test/CodeGen/PowerPC/redundant-copy-after-tail-dup.ll b/llvm/test/CodeGen/PowerPC/redundant-copy-after-tail-dup.ll index dd41abd..6aaf169 100644 --- a/llvm/test/CodeGen/PowerPC/redundant-copy-after-tail-dup.ll +++ b/llvm/test/CodeGen/PowerPC/redundant-copy-after-tail-dup.ll @@ -26,7 +26,8 @@ define dso_local i1 @t(%class.A* %this, i32 %color, i32 %vertex) local_unnamed_a ; CHECK-P9-NEXT: cmplwi r3, 2 ; CHECK-P9-NEXT: bge- cr0, .LBB0_6 ; CHECK-P9-NEXT: # %bb.3: # %land.lhs.true.1 -; CHECK-P9-NEXT: li r3, 0 +; CHECK-P9-NEXT: li r5, 0 +; CHECK-P9-NEXT: mr r3, r5 ; CHECK-P9-NEXT: blr ; CHECK-P9-NEXT: .LBB0_4: # %lor.lhs.false ; CHECK-P9-NEXT: cmplwi cr0, r4, 0 diff --git a/llvm/test/CodeGen/X86/mul-i512.ll b/llvm/test/CodeGen/X86/mul-i512.ll index a505046..40f6b09 100644 --- a/llvm/test/CodeGen/X86/mul-i512.ll +++ b/llvm/test/CodeGen/X86/mul-i512.ll @@ -153,8 +153,9 @@ define void @test_512(i512* %a, i512* %b, i512* %out) nounwind { ; X32-NEXT: movl %eax, {{[-0-9]+}}(%e{{[sb]}}p) # 4-byte Spill ; X32-NEXT: adcl $0, %edx ; X32-NEXT: movl %edx, {{[-0-9]+}}(%e{{[sb]}}p) # 4-byte Spill -; X32-NEXT: movl {{[0-9]+}}(%esp), %ecx -; X32-NEXT: movl 8(%ecx), %ebx +; X32-NEXT: movl {{[0-9]+}}(%esp), %esi +; X32-NEXT: movl %esi, %ecx +; X32-NEXT: movl 8(%esi), %ebx ; X32-NEXT: movl %ebx, %eax ; X32-NEXT: movl %ebx, {{[-0-9]+}}(%e{{[sb]}}p) # 4-byte Spill ; X32-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %esi # 4-byte Reload diff --git a/llvm/test/CodeGen/X86/umulo-128-legalisation-lowering.ll b/llvm/test/CodeGen/X86/umulo-128-legalisation-lowering.ll index 7ce16bb..4f26db8 100644 --- a/llvm/test/CodeGen/X86/umulo-128-legalisation-lowering.ll +++ b/llvm/test/CodeGen/X86/umulo-128-legalisation-lowering.ll @@ -98,8 +98,8 @@ define { i128, i8 } @muloti_test(i128 %l, i128 %r) unnamed_addr #0 { ; X86-NEXT: addl %esi, %ecx ; X86-NEXT: adcl $0, %ebp ; X86-NEXT: movl {{[0-9]+}}(%esp), %eax -; X86-NEXT: movl {{[0-9]+}}(%esp), %edx -; X86-NEXT: mull %edx +; X86-NEXT: movl {{[0-9]+}}(%esp), %esi +; X86-NEXT: mull %esi ; X86-NEXT: movl %edx, %esi ; X86-NEXT: addl %ecx, %eax ; X86-NEXT: movl %eax, {{[-0-9]+}}(%e{{[sb]}}p) # 4-byte Spill