From cb5a48b060fa8039f8f0812b2acb452336cc9a85 Mon Sep 17 00:00:00 2001 From: David Green Date: Wed, 20 Feb 2019 10:22:18 +0000 Subject: [PATCH] [Codegen] Remove dead flags on Physical Defs in machine cse We may leave behind incorrect dead flags on instructions that are CSE'd. Make sure we remove the dead flags on physical registers to prevent other incorrect code motion. Differential Revision: https://reviews.llvm.org/D58115 llvm-svn: 354443 --- llvm/lib/CodeGen/MachineCSE.cpp | 43 ++++---- .../CodeGen/Thumb/machine-cse-deadreg.mir | 103 ++++++++++++++++++ 2 files changed, 127 insertions(+), 19 deletions(-) create mode 100644 llvm/test/CodeGen/Thumb/machine-cse-deadreg.mir diff --git a/llvm/lib/CodeGen/MachineCSE.cpp b/llvm/lib/CodeGen/MachineCSE.cpp index f09288ec15c5c..ff15875af9d8b 100644 --- a/llvm/lib/CodeGen/MachineCSE.cpp +++ b/llvm/lib/CodeGen/MachineCSE.cpp @@ -94,6 +94,7 @@ namespace { ScopedHashTable; using ScopeType = ScopedHTType::ScopeTy; + using PhysDefVector = SmallVector, 2>; unsigned LookAheadLimit = 0; DenseMap ScopeMap; @@ -108,13 +109,11 @@ namespace { MachineBasicBlock::const_iterator E) const; bool hasLivePhysRegDefUses(const MachineInstr *MI, const MachineBasicBlock *MBB, - SmallSet &PhysRefs, - SmallVectorImpl &PhysDefs, - bool &PhysUseDef) const; + SmallSet &PhysRefs, + PhysDefVector &PhysDefs, bool &PhysUseDef) const; bool PhysRegDefsReach(MachineInstr *CSMI, MachineInstr *MI, - SmallSet &PhysRefs, - SmallVectorImpl &PhysDefs, - bool &NonLocal) const; + SmallSet &PhysRefs, + PhysDefVector &PhysDefs, bool &NonLocal) const; bool isCSECandidate(MachineInstr *MI); bool isProfitableToCSE(unsigned CSReg, unsigned Reg, MachineInstr *CSMI, MachineInstr *MI); @@ -255,9 +254,9 @@ static bool isCallerPreservedOrConstPhysReg(unsigned Reg, /// instruction does not uses a physical register. bool MachineCSE::hasLivePhysRegDefUses(const MachineInstr *MI, const MachineBasicBlock *MBB, - SmallSet &PhysRefs, - SmallVectorImpl &PhysDefs, - bool &PhysUseDef) const{ + SmallSet &PhysRefs, + PhysDefVector &PhysDefs, + bool &PhysUseDef) const { // First, add all uses to PhysRefs. for (const MachineOperand &MO : MI->operands()) { if (!MO.isReg() || MO.isDef()) @@ -277,7 +276,8 @@ bool MachineCSE::hasLivePhysRegDefUses(const MachineInstr *MI, // (which currently contains only uses), set the PhysUseDef flag. PhysUseDef = false; MachineBasicBlock::const_iterator I = MI; I = std::next(I); - for (const MachineOperand &MO : MI->operands()) { + for (const auto &MOP : llvm::enumerate(MI->operands())) { + const MachineOperand &MO = MOP.value(); if (!MO.isReg() || !MO.isDef()) continue; unsigned Reg = MO.getReg(); @@ -292,20 +292,21 @@ bool MachineCSE::hasLivePhysRegDefUses(const MachineInstr *MI, // common since this pass is run before livevariables. We can scan // forward a few instructions and check if it is obviously dead. if (!MO.isDead() && !isPhysDefTriviallyDead(Reg, I, MBB->end())) - PhysDefs.push_back(Reg); + PhysDefs.push_back(std::make_pair(MOP.index(), Reg)); } // Finally, add all defs to PhysRefs as well. for (unsigned i = 0, e = PhysDefs.size(); i != e; ++i) - for (MCRegAliasIterator AI(PhysDefs[i], TRI, true); AI.isValid(); ++AI) + for (MCRegAliasIterator AI(PhysDefs[i].second, TRI, true); AI.isValid(); + ++AI) PhysRefs.insert(*AI); return !PhysRefs.empty(); } bool MachineCSE::PhysRegDefsReach(MachineInstr *CSMI, MachineInstr *MI, - SmallSet &PhysRefs, - SmallVectorImpl &PhysDefs, + SmallSet &PhysRefs, + PhysDefVector &PhysDefs, bool &NonLocal) const { // For now conservatively returns false if the common subexpression is // not in the same basic block as the given instruction. The only exception @@ -319,7 +320,8 @@ bool MachineCSE::PhysRegDefsReach(MachineInstr *CSMI, MachineInstr *MI, return false; for (unsigned i = 0, e = PhysDefs.size(); i != e; ++i) { - if (MRI->isAllocatable(PhysDefs[i]) || MRI->isReserved(PhysDefs[i])) + if (MRI->isAllocatable(PhysDefs[i].second) || + MRI->isReserved(PhysDefs[i].second)) // Avoid extending live range of physical registers if they are //allocatable or reserved. return false; @@ -535,7 +537,7 @@ bool MachineCSE::ProcessBlock(MachineBasicBlock *MBB) { // It's also not safe if the instruction uses physical registers. bool CrossMBBPhysDef = false; SmallSet PhysRefs; - SmallVector PhysDefs; + PhysDefVector PhysDefs; bool PhysUseDef = false; if (FoundCSE && hasLivePhysRegDefUses(MI, MBB, PhysRefs, PhysDefs, PhysUseDef)) { @@ -634,6 +636,9 @@ bool MachineCSE::ProcessBlock(MachineBasicBlock *MBB) { // we should make sure it is not dead at CSMI. for (unsigned ImplicitDefToUpdate : ImplicitDefsToUpdate) CSMI->getOperand(ImplicitDefToUpdate).setIsDead(false); + for (auto PhysDef : PhysDefs) + if (!MI->getOperand(PhysDef.first).isDead()) + CSMI->getOperand(PhysDef.first).setIsDead(false); // Go through implicit defs of CSMI and MI, and clear the kill flags on // their uses in all the instructions between CSMI and MI. @@ -662,9 +667,9 @@ bool MachineCSE::ProcessBlock(MachineBasicBlock *MBB) { // Add physical register defs now coming in from a predecessor to MBB // livein list. while (!PhysDefs.empty()) { - unsigned LiveIn = PhysDefs.pop_back_val(); - if (!MBB->isLiveIn(LiveIn)) - MBB->addLiveIn(LiveIn); + auto LiveIn = PhysDefs.pop_back_val(); + if (!MBB->isLiveIn(LiveIn.second)) + MBB->addLiveIn(LiveIn.second); } ++NumCrossBBCSEs; } diff --git a/llvm/test/CodeGen/Thumb/machine-cse-deadreg.mir b/llvm/test/CodeGen/Thumb/machine-cse-deadreg.mir new file mode 100644 index 0000000000000..30ca5513a456d --- /dev/null +++ b/llvm/test/CodeGen/Thumb/machine-cse-deadreg.mir @@ -0,0 +1,103 @@ +# RUN: llc -mtriple thumbv6m-arm-none-eabi -run-pass=machine-cse -verify-machineinstrs -o - %s | FileCheck %s + +--- | + target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64" + target triple = "thumbv6m-arm-none-eabi" + + define i32 @funca(i64 %l1) { + entry: + %l2 = icmp ult i64 %l1, -177673816660004267 + %l3 = add i64 %l1, 401100203 + %rem.i = select i1 %l2, i64 %l1, i64 %l3 + %conv = trunc i64 %rem.i to i32 + ret i32 %conv + } + + define i32 @funcb(i64 %l1) { ret i32 0 } + +... +--- +name: funca +tracksRegLiveness: true +liveins: + - { reg: '$r0', virtual-reg: '%0' } + - { reg: '$r1', virtual-reg: '%1' } +constants: + - id: 0 + value: i32 401100203 + alignment: 4 + isTargetSpecific: false + - id: 1 + value: i32 41367909 + alignment: 4 + isTargetSpecific: false +body: | + bb.0.entry: + successors: %bb.1(0x40000000), %bb.2(0x40000000) + liveins: $r0, $r1 + + %1:tgpr = COPY $r1 + %0:tgpr = COPY $r0 + %2:tgpr = tLDRpci %const.0, 14, $noreg :: (load 4 from constant-pool) + %3:tgpr, dead $cpsr = tADDrr %0, %2, 14, $noreg + %4:tgpr = tLDRpci %const.1, 14, $noreg :: (load 4 from constant-pool) + %5:tgpr, $cpsr = tADDrr %0, %2, 14, $noreg + %6:tgpr, $cpsr = tADC %1, killed %4, 14, $noreg, implicit $cpsr + tBcc %bb.2, 3, $cpsr + + bb.1.entry: + successors: %bb.2(0x80000000) + + bb.2.entry: + %7:tgpr = PHI %3, %bb.1, %0, %bb.0 + $r0 = COPY %7 + tBX_RET 14, $noreg, implicit $r0 + +# CHECK-LABEL: name: funca +# cpsr def must not be dead +# CHECK: %3:tgpr, $cpsr = tADDrr %0, %2 +# CHECK-NOT: %5:tgpr, $cpsr = tADDrr %0, %2 + +... +--- +name: funcb +tracksRegLiveness: true +liveins: + - { reg: '$r0', virtual-reg: '%0' } + - { reg: '$r1', virtual-reg: '%1' } +constants: + - id: 0 + value: i32 401100203 + alignment: 4 + isTargetSpecific: false + - id: 1 + value: i32 41367909 + alignment: 4 + isTargetSpecific: false +body: | + bb.0: + successors: %bb.1(0x40000000), %bb.2(0x40000000) + liveins: $r0, $r1 + + %1:tgpr = COPY $r1 + %0:tgpr = COPY $r0 + %2:tgpr = tLDRpci %const.0, 14, $noreg :: (load 4 from constant-pool) + %3:tgpr, dead $cpsr = tADDrr %0, %2, 14, $noreg + %4:tgpr = tLDRpci %const.1, 14, $noreg :: (load 4 from constant-pool) + %5:tgpr, dead $cpsr = tADDrr %0, %2, 14, $noreg + %6:tgpr, $cpsr = tADDrr %1, killed %4, 14, $noreg + tBcc %bb.2, 3, $cpsr + + bb.1: + successors: %bb.2(0x80000000) + + bb.2: + %7:tgpr = PHI %3, %bb.1, %0, %bb.0 + $r0 = COPY %7 + tBX_RET 14, $noreg, implicit $r0 + +# CHECK-LABEL: name: funcb +# cpsr def should be dead +# CHECK: %3:tgpr, dead $cpsr = tADDrr %0, %2 +# CHECK-NOT: %5:tgpr, dead $cpsr = tADDrr %0, %2 +...