llvm dev - Feb 2012 - [LLVMdev] [llvm-commits] [PATCH] MachineRegisterInfo: Don't emit the same livein copy more than once

On Mon, Feb 13, 2012 at 10:17:11PM -0800, Lang Hames
wrote:
> Hi Tom,
> 
> I'm pretty sure this function should only ever be called once, by
> SelectionDAG. Do you know where the second call is coming from in your
code?
> 
> Cheers,
> Lang.
Hi Lang,

I was calling EmitLiveInCopies() from one of my backend specific passes.
If the function can only be called once, then I'll just try to merge
that pass with into the SelectionDAG.

Thanks,
Tom
> 
> On Mon, Feb 13, 2012 at 7:03 PM, Stellard, Thomas <Tom.Stellard at
amd.com>wrote:
> 
> > This patch seems to have been lost on the llvm-commits mailing list.
> >  Would someone be able to review it?
> >
> > Thanks,
> > Tom
> > ________________________________________
> > From: llvm-commits-bounces at cs.uiuc.edu [llvm-commits-bounces at
cs.uiuc.edu]
> > on behalf of Tom Stellard [thomas.stellard at amd.com]
> > Sent: Friday, February 03, 2012 1:55 PM
> > To: llvm-commits at cs.uiuc.edu
> > Subject: Re: [llvm-commits] [PATCH] MachineRegisterInfo: Don't
emit the
> > same livein copy more than once
> >
> > On Fri, Jan 27, 2012 at 02:56:03PM -0500, Tom Stellard wrote:
> > > ---
> > >
> > > Is MachineRegisterInfo::EmitLiveInCopies() only meant to be
called once
> > > per compile?  If I call it more than once, it emits duplicate
copies
> > > which causes the live interval analysis to fail.
> > >
> > >  lib/CodeGen/MachineRegisterInfo.cpp |    4 +++-
> > >  1 files changed, 3 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/lib/CodeGen/MachineRegisterInfo.cpp
> > b/lib/CodeGen/MachineRegisterInfo.cpp
> > > index 266ebf6..fc787f2 100644
> > > --- a/lib/CodeGen/MachineRegisterInfo.cpp
> > > +++ b/lib/CodeGen/MachineRegisterInfo.cpp
> > > @@ -227,7 +227,9 @@
> > MachineRegisterInfo::EmitLiveInCopies(MachineBasicBlock *EntryMBB,
> > >          // complicated by the debug info code for arguments.
> > >          LiveIns.erase(LiveIns.begin() + i);
> > >          --i; --e;
> > > -      } else {
> > > +        //Make sure we don't emit the same livein copies
twice, in case
> > this
> > > +        //function is called more than once.
> > > +      } else if (def_empty(LiveIns[i].second)) {
> > >          // Emit a copy.
> > >          BuildMI(*EntryMBB, EntryMBB->begin(), DebugLoc(),
> > >                  TII.get(TargetOpcode::COPY), LiveIns[i].second)
> > > --
> > > 1.7.6.4
> > >
> > >
> >
> > Reposting this as a diff that can be applied via patch -P0 for SVN
> > users.
> >
> > -Tom
> >
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >

Hi Tom,

As far as I can tell EmitLiveInCopies is just there to handle physreg
arguments and return values. Is there any reason for these to change late
in your backend?

- Lang.


On Tue, Feb 14, 2012 at 7:22 AM, Tom Stellard <thomas.stellard at
amd.com>wrote:
> On Mon, Feb 13, 2012 at 10:17:11PM -0800, Lang Hames wrote:
> > Hi Tom,
> >
> > I'm pretty sure this function should only ever be called once, by
> > SelectionDAG. Do you know where the second call is coming from in your
> code?
> >
> > Cheers,
> > Lang.
>
> Hi Lang,
>
> I was calling EmitLiveInCopies() from one of my backend specific passes.
> If the function can only be called once, then I'll just try to merge
> that pass with into the SelectionDAG.
>
> Thanks,
> Tom
>
> >
> > On Mon, Feb 13, 2012 at 7:03 PM, Stellard, Thomas <Tom.Stellard at
amd.com
> >wrote:
> >
> > > This patch seems to have been lost on the llvm-commits mailing
list.
> > >  Would someone be able to review it?
> > >
> > > Thanks,
> > > Tom
> > > ________________________________________
> > > From: llvm-commits-bounces at cs.uiuc.edu [
> llvm-commits-bounces at cs.uiuc.edu]
> > > on behalf of Tom Stellard [thomas.stellard at amd.com]
> > > Sent: Friday, February 03, 2012 1:55 PM
> > > To: llvm-commits at cs.uiuc.edu
> > > Subject: Re: [llvm-commits] [PATCH] MachineRegisterInfo:
Don't emit the
> > > same livein copy more than once
> > >
> > > On Fri, Jan 27, 2012 at 02:56:03PM -0500, Tom Stellard wrote:
> > > > ---
> > > >
> > > > Is MachineRegisterInfo::EmitLiveInCopies() only meant to be
called
> once
> > > > per compile?  If I call it more than once, it emits
duplicate copies
> > > > which causes the live interval analysis to fail.
> > > >
> > > >  lib/CodeGen/MachineRegisterInfo.cpp |    4 +++-
> > > >  1 files changed, 3 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/lib/CodeGen/MachineRegisterInfo.cpp
> > > b/lib/CodeGen/MachineRegisterInfo.cpp
> > > > index 266ebf6..fc787f2 100644
> > > > --- a/lib/CodeGen/MachineRegisterInfo.cpp
> > > > +++ b/lib/CodeGen/MachineRegisterInfo.cpp
> > > > @@ -227,7 +227,9 @@
> > > MachineRegisterInfo::EmitLiveInCopies(MachineBasicBlock
*EntryMBB,
> > > >          // complicated by the debug info code for
arguments.
> > > >          LiveIns.erase(LiveIns.begin() + i);
> > > >          --i; --e;
> > > > -      } else {
> > > > +        //Make sure we don't emit the same livein
copies twice, in
> case
> > > this
> > > > +        //function is called more than once.
> > > > +      } else if (def_empty(LiveIns[i].second)) {
> > > >          // Emit a copy.
> > > >          BuildMI(*EntryMBB, EntryMBB->begin(),
DebugLoc(),
> > > >                  TII.get(TargetOpcode::COPY),
LiveIns[i].second)
> > > > --
> > > > 1.7.6.4
> > > >
> > > >
> > >
> > > Reposting this as a diff that can be applied via patch -P0 for
SVN
> > > users.
> > >
> > > -Tom
> > >
> > >
> > > _______________________________________________
> > > LLVM Developers mailing list
> > > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120214/6eba3eed/attachment.html>

On Tue, Feb 14, 2012 at 06:33:34PM -0800, Lang Hames
wrote:
> Hi Tom,
> 
> As far as I can tell EmitLiveInCopies is just there to handle physreg
> arguments and return values. Is there any reason for these to change late
> in your backend?
>
No, on our target (AMD HD2XXX - HD6XXX GPUs) before the program begins,
some values are preloaded into registers.  We have special intrinsics
for reading these values (e.g.  llvm.R600.read_reg(i32 reg_idx) ), and
we just have a pass that converts these intrinsics to livein registers
and then calls EmitLiveInCopies().

-Tom
> - Lang.
> 
> 
> On Tue, Feb 14, 2012 at 7:22 AM, Tom Stellard <thomas.stellard at
amd.com>wrote:
> 
> > On Mon, Feb 13, 2012 at 10:17:11PM -0800, Lang Hames wrote:
> > > Hi Tom,
> > >
> > > I'm pretty sure this function should only ever be called
once, by
> > > SelectionDAG. Do you know where the second call is coming from in
your
> > code?
> > >
> > > Cheers,
> > > Lang.
> >
> > Hi Lang,
> >
> > I was calling EmitLiveInCopies() from one of my backend specific
passes.
> > If the function can only be called once, then I'll just try to
merge
> > that pass with into the SelectionDAG.
> >
> > Thanks,
> > Tom
> >
> > >
> > > On Mon, Feb 13, 2012 at 7:03 PM, Stellard, Thomas
<Tom.Stellard at amd.com
> > >wrote:
> > >
> > > > This patch seems to have been lost on the llvm-commits
mailing list.
> > > >  Would someone be able to review it?
> > > >
> > > > Thanks,
> > > > Tom
> > > > ________________________________________
> > > > From: llvm-commits-bounces at cs.uiuc.edu [
> > llvm-commits-bounces at cs.uiuc.edu]
> > > > on behalf of Tom Stellard [thomas.stellard at amd.com]
> > > > Sent: Friday, February 03, 2012 1:55 PM
> > > > To: llvm-commits at cs.uiuc.edu
> > > > Subject: Re: [llvm-commits] [PATCH] MachineRegisterInfo:
Don't emit the
> > > > same livein copy more than once
> > > >
> > > > On Fri, Jan 27, 2012 at 02:56:03PM -0500, Tom Stellard
wrote:
> > > > > ---
> > > > >
> > > > > Is MachineRegisterInfo::EmitLiveInCopies() only meant
to be called
> > once
> > > > > per compile?  If I call it more than once, it emits
duplicate copies
> > > > > which causes the live interval analysis to fail.
> > > > >
> > > > >  lib/CodeGen/MachineRegisterInfo.cpp |    4 +++-
> > > > >  1 files changed, 3 insertions(+), 1 deletions(-)
> > > > >
> > > > > diff --git a/lib/CodeGen/MachineRegisterInfo.cpp
> > > > b/lib/CodeGen/MachineRegisterInfo.cpp
> > > > > index 266ebf6..fc787f2 100644
> > > > > --- a/lib/CodeGen/MachineRegisterInfo.cpp
> > > > > +++ b/lib/CodeGen/MachineRegisterInfo.cpp
> > > > > @@ -227,7 +227,9 @@
> > > > MachineRegisterInfo::EmitLiveInCopies(MachineBasicBlock
*EntryMBB,
> > > > >          // complicated by the debug info code for
arguments.
> > > > >          LiveIns.erase(LiveIns.begin() + i);
> > > > >          --i; --e;
> > > > > -      } else {
> > > > > +        //Make sure we don't emit the same livein
copies twice, in
> > case
> > > > this
> > > > > +        //function is called more than once.
> > > > > +      } else if (def_empty(LiveIns[i].second)) {
> > > > >          // Emit a copy.
> > > > >          BuildMI(*EntryMBB, EntryMBB->begin(),
DebugLoc(),
> > > > >                  TII.get(TargetOpcode::COPY),
LiveIns[i].second)
> > > > > --
> > > > > 1.7.6.4
> > > > >
> > > > >
> > > >
> > > > Reposting this as a diff that can be applied via patch -P0
for SVN
> > > > users.
> > > >
> > > > -Tom
> > > >
> > > >
> > > > _______________________________________________
> > > > LLVM Developers mailing list
> > > > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > > >
> >
> >