[llvm-commits] [llvm] r80654 - in /llvm/trunk: lib/CodeGen/TwoAddressInstructionPass.cpp test/CodeGen/ARM/2009-08-31-TwoRegShuffle.ll
Evan Cheng
evan.cheng at apple.com
Tue Sep 1 00:37:53 PDT 2009
Hi Bob,
I am taking issues with this patch. The two-address pass is starting
to get too complicated (my fault). I've been thinking about
restructuring the code. This patch is making the code even harder to
read. It also potentially calls removeVirtualRegisterKilled() multiple
times for each source register. Each of the call ends up scanning the
entire operand list. That seems inefficient to me. Also, what happens
to the code the calls LV->removeVirtualRegisterDead(regB, mi)?
if (KillMO) {
if (!FirstKeptMO) {
// All uses of regB are being replaced; move the kill to prevMI.
if (LV && LV->removeVirtualRegisterKilled(regB, mi))
When KillMO is not null, it shouldn't be necessary to call
removeVirtualRegisterKilled, right?
Also notice we are scanning the operands twice:
for (unsigned i = 0, e = mi->getNumOperands(); i != e; ++i) {
...
}
// Replace uses of regB with regA.
for (unsigned i = 0, e = mi->getNumOperands(); i != e; ++i) {
...
}
Please scan the operands just once. If it sees any other source
operand of the same virtual register tied to another def, then it
should not update the other operands of the same virtual register. The
next iteration will deal with them.
As for the kill marker, it should move it to the previous instruction
the first time it is processed. All you have to ensure is the later
iteration does not insert any instruction between the kill marker and
the mi being processed. That should be pretty straight forward. Just
add a iterator which is the instruction insertion location. The first
time you insert a copy, it should then be moved to the copy
instruction, etc. e.g.
a, b = op c<kill>, c // insert pos
=>
a = c<kill> // insert pos
a, b = op a, c
=>
b = c
a = c<kill>
a, b = op a, b
Since you are working on this code, could be please clean it up a bit.
Can you factor this if statement:
if (mi->getOperand(ti).isDead() &&
isSafeToDelete(mi, regB, TII, Kills)) {
to a separate function?
Thanks,
Evan
On Aug 31, 2009, at 9:18 PM, Bob Wilson wrote:
> Author: bwilson
> Date: Mon Aug 31 23:18:40 2009
> New Revision: 80654
>
> URL: http://llvm.org/viewvc/llvm-project?rev=80654&view=rev
> Log:
> Fix pr4843: When an instruction has multiple destination registers
> that are
> tied to different source registers, the TwoAddressInstructionPass
> needs to
> be smarter. Change it to check before replacing a source register
> whether
> that source register is tied to a different destination register,
> and if so,
> defer handling it until a subsequent iteration.
>
> Added:
> llvm/trunk/test/CodeGen/ARM/2009-08-31-TwoRegShuffle.ll
> Modified:
> llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp
>
> Modified: llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp?rev=80654&r1=80653&r2=80654&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp (original)
> +++ llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp Mon Aug 31
> 23:18:40 2009
> @@ -968,23 +968,67 @@
> // Update DistanceMap.
> DistanceMap.insert(std::make_pair(prevMI, Dist));
> DistanceMap[mi] = ++Dist;
> +
> + // Scan the operands to find: (1) the use operand that
> kills regB (if
> + // any); (2) whether the kill operand is being replaced by
> regA on
> + // this iteration; and (3) the first use of regB that is
> not being
> + // replaced on this iteration. A use of regB will not
> replaced if it
> + // is tied to a different destination register and will be
> handled on
> + // a later iteration.
> + MachineOperand *KillMO = NULL;
> + MachineOperand *FirstKeptMO = NULL;
> + bool KillMOKept = false;
> + for (unsigned i = 0, e = mi->getNumOperands(); i != e; ++i) {
> + MachineOperand &MO = mi->getOperand(i);
> + if (MO.isReg() && MO.getReg() == regB && MO.isUse()) {
>
> - // Update live variables for regB.
> - if (LV) {
> - if (LV->removeVirtualRegisterKilled(regB, mi))
> - LV->addVirtualRegisterKilled(regB, prevMI);
> + // Check if this operand is tied to a different
> destination.
> + bool isKept = false;
> + unsigned dsti = 0;
> + if (mi->isRegTiedToDefOperand(i, &dsti) && dsti != ti) {
> + isKept = true;
> + if (!FirstKeptMO)
> + FirstKeptMO = &MO;
> + }
> +
> + if (MO.isKill()) {
> + KillMO = &MO;
> + KillMOKept = isKept;
> + }
> + }
> + }
>
> - if (LV->removeVirtualRegisterDead(regB, mi))
> - LV->addVirtualRegisterDead(regB, prevMI);
> + // Update live variables for regB.
> + if (KillMO) {
> + if (!FirstKeptMO) {
> + // All uses of regB are being replaced; move the kill
> to prevMI.
> + if (LV && LV->removeVirtualRegisterKilled(regB, mi))
> + LV->addVirtualRegisterKilled(regB, prevMI);
> + } else {
> + if (!KillMOKept) {
> + // The kill marker is on an operand being replaced,
> but there
> + // are other uses of regB remaining. Move the kill
> marker to
> + // one of them.
> + KillMO->setIsKill(false);
> + FirstKeptMO->setIsKill(true);
> + }
> + }
> }
>
> DEBUG(errs() << "\t\tprepend:\t" << *prevMI);
> -
> - // Replace all occurences of regB with regA.
> +
> + // Replace uses of regB with regA.
> for (unsigned i = 0, e = mi->getNumOperands(); i != e; ++i) {
> - if (mi->getOperand(i).isReg() &&
> - mi->getOperand(i).getReg() == regB)
> - mi->getOperand(i).setReg(regA);
> + MachineOperand &MO = mi->getOperand(i);
> + if (MO.isReg() && MO.getReg() == regB && MO.isUse()) {
> +
> + // Skip operands that are tied to other register
> definitions.
> + unsigned dsti = 0;
> + if (mi->isRegTiedToDefOperand(i, &dsti) && dsti != ti)
> + continue;
> +
> + MO.setReg(regA);
> + }
> }
>
> assert(mi->getOperand(ti).isDef() && mi-
> >getOperand(si).isUse());
>
> Added: llvm/trunk/test/CodeGen/ARM/2009-08-31-TwoRegShuffle.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2009-08-31-TwoRegShuffle.ll?rev=80654&view=auto
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/test/CodeGen/ARM/2009-08-31-TwoRegShuffle.ll (added)
> +++ llvm/trunk/test/CodeGen/ARM/2009-08-31-TwoRegShuffle.ll Mon Aug
> 31 23:18:40 2009
> @@ -0,0 +1,9 @@
> +; RUN: llvm-as < %s | llc -march=arm -mattr=+neon | FileCheck %s
> +; pr4843
> +define <4 x i16> @v2regbug(<4 x i16>* %B) nounwind {
> +;CHECK: v2regbug:
> +;CHECK: vzip.16
> + %tmp1 = load <4 x i16>* %B
> + %tmp2 = shufflevector <4 x i16> %tmp1, <4 x i16> undef, <4 x
> i32><i32 0, i32 0, i32 1, i32 1>
> + ret <4 x i16> %tmp2
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list