[llvm-commits] [llvm] r167855 - in /llvm/trunk: lib/CodeGen/MachineCSE.cpp test/CodeGen/PowerPC/2012-10-11-dynalloc.ll
Hal Finkel
hfinkel at anl.gov
Fri Nov 16 14:59:52 PST 2012
Evan,
Can this fix be merged into the release?
Thanks again,
Hal
----- Original Message -----
> From: "Ulrich Weigand" <ulrich.weigand at de.ibm.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Tuesday, November 13, 2012 12:40:58 PM
> Subject: [llvm-commits] [llvm] r167855 - in /llvm/trunk: lib/CodeGen/MachineCSE.cpp
> test/CodeGen/PowerPC/2012-10-11-dynalloc.ll
>
> Author: uweigand
> Date: Tue Nov 13 12:40:58 2012
> New Revision: 167855
>
> URL: http://llvm.org/viewvc/llvm-project?rev=167855&view=rev
> Log:
> Do not consider a machine instruction that uses and defines the same
> physical register as candidate for common subexpression elimination
> in MachineCSE.
>
> This fixes a bug on PowerPC in MultiSource/Applications/oggenc/oggenc
> caused by MachineCSE invalidly merging two separate DYNALLOC insns.
>
> Added:
> llvm/trunk/test/CodeGen/PowerPC/2012-10-11-dynalloc.ll
> Modified:
> llvm/trunk/lib/CodeGen/MachineCSE.cpp
>
> Modified: llvm/trunk/lib/CodeGen/MachineCSE.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineCSE.cpp?rev=167855&r1=167854&r2=167855&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineCSE.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineCSE.cpp Tue Nov 13 12:40:58 2012
> @@ -84,7 +84,8 @@
> bool hasLivePhysRegDefUses(const MachineInstr *MI,
> const MachineBasicBlock *MBB,
> SmallSet<unsigned,8> &PhysRefs,
> - SmallVector<unsigned,2> &PhysDefs)
> const;
> + SmallVector<unsigned,2> &PhysDefs,
> + bool &PhysUseDef) const;
> bool PhysRegDefsReach(MachineInstr *CSMI, MachineInstr *MI,
> SmallSet<unsigned,8> &PhysRefs,
> SmallVector<unsigned,2> &PhysDefs,
> @@ -194,31 +195,52 @@
> bool MachineCSE::hasLivePhysRegDefUses(const MachineInstr *MI,
> const MachineBasicBlock *MBB,
> SmallSet<unsigned,8>
> &PhysRefs,
> - SmallVector<unsigned,2>
> &PhysDefs) const{
> - MachineBasicBlock::const_iterator I = MI; I = llvm::next(I);
> + SmallVector<unsigned,2>
> &PhysDefs,
> + bool &PhysUseDef) const{
> + // First, add all uses to PhysRefs.
> for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
> const MachineOperand &MO = MI->getOperand(i);
> - if (!MO.isReg())
> + if (!MO.isReg() || MO.isDef())
> continue;
> unsigned Reg = MO.getReg();
> if (!Reg)
> continue;
> if (TargetRegisterInfo::isVirtualRegister(Reg))
> continue;
> - // If the def is dead, it's ok. But the def may not marked
> "dead". That's
> - // common since this pass is run before livevariables. We can
> scan
> - // forward a few instructions and check if it is obviously dead.
> - if (MO.isDef() &&
> - (MO.isDead() || isPhysDefTriviallyDead(Reg, I, MBB->end())))
> - continue;
> // Reading constant physregs is ok.
> if (!MRI->isConstantPhysReg(Reg, *MBB->getParent()))
> for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid();
> ++AI)
> PhysRefs.insert(*AI);
> - if (MO.isDef())
> + }
> +
> + // Next, collect all defs into PhysDefs. If any is already in
> PhysRefs
> + // (which currently contains only uses), set the PhysUseDef flag.
> + PhysUseDef = false;
> + MachineBasicBlock::const_iterator I = MI; I = llvm::next(I);
> + for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
> + const MachineOperand &MO = MI->getOperand(i);
> + if (!MO.isReg() || !MO.isDef())
> + continue;
> + unsigned Reg = MO.getReg();
> + if (!Reg)
> + continue;
> + if (TargetRegisterInfo::isVirtualRegister(Reg))
> + continue;
> + // Check against PhysRefs even if the def is "dead".
> + if (PhysRefs.count(Reg))
> + PhysUseDef = true;
> + // If the def is dead, it's ok. But the def may not marked
> "dead". That's
> + // 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);
> }
>
> + // 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)
> + PhysRefs.insert(*AI);
> +
> return !PhysRefs.empty();
> }
>
> @@ -459,16 +481,22 @@
> bool CrossMBBPhysDef = false;
> SmallSet<unsigned, 8> PhysRefs;
> SmallVector<unsigned, 2> PhysDefs;
> - if (FoundCSE && hasLivePhysRegDefUses(MI, MBB, PhysRefs,
> PhysDefs)) {
> + bool PhysUseDef = false;
> + if (FoundCSE && hasLivePhysRegDefUses(MI, MBB, PhysRefs,
> + PhysDefs, PhysUseDef)) {
> FoundCSE = false;
>
> // ... Unless the CS is local or is in the sole predecessor
> block
> // and it also defines the physical register which is not
> clobbered
> // in between and the physical register uses were not
> clobbered.
> - unsigned CSVN = VNT.lookup(MI);
> - MachineInstr *CSMI = Exps[CSVN];
> - if (PhysRegDefsReach(CSMI, MI, PhysRefs, PhysDefs,
> CrossMBBPhysDef))
> - FoundCSE = true;
> + // This can never be the case if the instruction both uses and
> + // defines the same physical register, which was detected
> above.
> + if (!PhysUseDef) {
> + unsigned CSVN = VNT.lookup(MI);
> + MachineInstr *CSMI = Exps[CSVN];
> + if (PhysRegDefsReach(CSMI, MI, PhysRefs, PhysDefs,
> CrossMBBPhysDef))
> + FoundCSE = true;
> + }
> }
>
> if (!FoundCSE) {
>
> Added: llvm/trunk/test/CodeGen/PowerPC/2012-10-11-dynalloc.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/2012-10-11-dynalloc.ll?rev=167855&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/PowerPC/2012-10-11-dynalloc.ll (added)
> +++ llvm/trunk/test/CodeGen/PowerPC/2012-10-11-dynalloc.ll Tue Nov 13
> 12:40:58 2012
> @@ -0,0 +1,18 @@
> +; RUN: llc < %s | FileCheck %s
> +target datalayout =
> "E-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v128:128:128-n32:64"
> +target triple = "powerpc64-unknown-linux-gnu"
> +
> +define void @test(i64 %n) nounwind {
> +entry:
> + %0 = alloca i8, i64 %n, align 1
> + %1 = alloca i8, i64 %n, align 1
> + call void @use(i8* %0, i8* %1) nounwind
> + ret void
> +}
> +
> +declare void @use(i8*, i8*)
> +
> +; Check we actually have two instances of dynamic stack allocation,
> +; identified by the stdux used to update the back-chain link.
> +; CHECK: stdux
> +; CHECK: stdux
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list