[llvm-commits] [llvm] r167855 - in /llvm/trunk: lib/CodeGen/MachineCSE.cpp test/CodeGen/PowerPC/2012-10-11-dynalloc.ll
Evan Cheng
evan.cheng at apple.com
Fri Nov 16 16:02:40 PST 2012
Ok.
Evan
On Nov 16, 2012, at 2:59 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> 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