Index: llvm/trunk/lib/CodeGen/BranchFolding.cpp =================================================================== --- llvm/trunk/lib/CodeGen/BranchFolding.cpp +++ llvm/trunk/lib/CodeGen/BranchFolding.cpp @@ -1850,8 +1850,8 @@ return false; bool HasDups = false; - SmallVector LocalDefs; - SmallSet LocalDefsSet; + SmallVector LocalDefs, LocalKills; + SmallSet ActiveDefsSet, AllDefsSet; MachineBasicBlock::iterator TIB = TBB->begin(); MachineBasicBlock::iterator FIB = FBB->begin(); MachineBasicBlock::iterator TIE = TBB->end(); @@ -1905,7 +1905,7 @@ IsSafe = false; break; } - } else if (!LocalDefsSet.count(Reg)) { + } else if (!ActiveDefsSet.count(Reg)) { if (Defs.count(Reg)) { // Use is defined by the instruction at the point of insertion. IsSafe = false; @@ -1925,18 +1925,22 @@ if (!TIB->isSafeToMove(nullptr, DontMoveAcrossStore)) break; - // Remove kills from LocalDefsSet, these registers had short live ranges. + // Remove kills from ActiveDefsSet, these registers had short live ranges. for (const MachineOperand &MO : TIB->operands()) { if (!MO.isReg() || !MO.isUse() || !MO.isKill()) continue; unsigned Reg = MO.getReg(); - if (!Reg || !LocalDefsSet.count(Reg)) + if (!Reg) + continue; + if (!AllDefsSet.count(Reg)) { + LocalKills.push_back(Reg); continue; + } if (TargetRegisterInfo::isPhysicalRegister(Reg)) { for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) - LocalDefsSet.erase(*AI); + ActiveDefsSet.erase(*AI); } else { - LocalDefsSet.erase(Reg); + ActiveDefsSet.erase(Reg); } } @@ -1948,7 +1952,8 @@ if (!Reg || TargetRegisterInfo::isVirtualRegister(Reg)) continue; LocalDefs.push_back(Reg); - addRegAndItsAliases(Reg, TRI, LocalDefsSet); + addRegAndItsAliases(Reg, TRI, ActiveDefsSet); + addRegAndItsAliases(Reg, TRI, AllDefsSet); } HasDups = true; @@ -1963,17 +1968,22 @@ FBB->erase(FBB->begin(), FIB); // Update livein's. - bool AddedLiveIns = false; + bool ChangedLiveIns = false; for (unsigned i = 0, e = LocalDefs.size(); i != e; ++i) { unsigned Def = LocalDefs[i]; - if (LocalDefsSet.count(Def)) { + if (ActiveDefsSet.count(Def)) { TBB->addLiveIn(Def); FBB->addLiveIn(Def); - AddedLiveIns = true; + ChangedLiveIns = true; } } + for (unsigned K : LocalKills) { + TBB->removeLiveIn(K); + FBB->removeLiveIn(K); + ChangedLiveIns = true; + } - if (AddedLiveIns) { + if (ChangedLiveIns) { TBB->sortUniqueLiveIns(); FBB->sortUniqueLiveIns(); } Index: llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir =================================================================== --- llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir +++ llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir @@ -0,0 +1,59 @@ +# RUN: llc -march=hexagon -run-pass branch-folder -run-pass if-converter -verify-machineinstrs %s -o - | FileCheck %s + +# The hoisting of common instructions from successors could cause registers +# to no longer be live-in in the successor blocks. The liveness was updated +# to include potential new live-in registres, but not to remove registers +# that were no longer live-in. +# This could cause if-converter to generate incorrect code. +# +# In this testcase, the "r1 = A2_sxth r0" was hoisted, and since r0 +# was killed, it was no longer live-in in either successor. The if-converter +# then created code, where the first predicated instruction has incorrect +# implicit use of r0: +# +# BB#0: +# Live Ins: %R0 +# %R1 = A2_sxth %R0 ; hoisted, kills r0 +# A2_nop %P0 +# %R0 = C2_cmoveit %P0, 2, %R0 ; predicated A2_tfrsi +# %R0 = C2_cmoveif %P0, 1, %R0 ; predicated A2_tfrsi +# %R0 = A2_add %R0, %R1 +# J2_jumpr %R31, %PC +# + +# CHECK: %r1 = A2_sxth killed %r0 +# CHECK: %r0 = C2_cmoveit %p0, 2 +# CHECK-NOT: implicit-def %r0 +# CHECK: %r0 = C2_cmoveif %p0, 1, implicit %r0 + +--- +name: fred +tracksRegLiveness: true + +body: | + bb.0: + liveins: %r0 + successors: %bb.1, %bb.2 + + A2_nop implicit-def %p0 + J2_jumpt killed %p0, %bb.2, implicit-def dead %pc + + bb.1: + successors: %bb.3 + liveins: %r0 + %r1 = A2_sxth killed %r0 + %r0 = A2_tfrsi 1 + J2_jump %bb.3, implicit-def %pc + + bb.2: + successors: %bb.3 + liveins: %r0 + %r1 = A2_sxth killed %r0 + %r0 = A2_tfrsi 2 + + bb.3: + liveins: %r0, %r1 + %r0 = A2_add killed %r0, killed %r1 + J2_jumpr %r31, implicit-def dead %pc +... +