Fix the aggressive anti-dep breaker's subregister definition handling

Tue Jan 27 01:10:33 PST 2015

Yes, patch fixes the problem.
Thanks a lot!

FYI
Using your patch:

Anti:   %R4_D<def> = ASRDrr %R0_D<kill>, %R6_S
 Def Groups: R4_D=g213->g215(via R4_S)->g214(via R4_L)->g216(via
R5_S)->g216(via R4_L)->g217(via R5_L)
 Use Groups: R0_D=g0->g218(last-use) R1_L->g219(last-use)
R6_S=g204->g220(last-use)
Anti:   %R0_L<def> = ANDLri %R0_L<kill>, 2047, pred:8, pred:%noreg
 Def Groups: R0_L=g208->g209(via R0_S)->g218(via R0_D)->g210(via
R1_S)->g210(via R0_D)
 Antidep reg: R0_L (real dependency)
 Use Groups: R0_L=g210
Anti:   %R1_L<def> = ANDLri %R1_L<kill>, 2047, pred:8, pred:%noreg
 Def Groups: R1_L=g219->g210(via R0_D)
 Antidep reg: R1_L (real dependency)
 Use Groups: R1_L=g210
Anti:   %R0_L<def> = LSRLrr %R2_L, %R0_S, pred:8, pred:%noreg,
%R0_L<imp-use,kill>
 Def Groups: R0_L=g210->g210(via R0_D)->g210(via R0_D)
 Antidep reg: R0_L Use Groups: R2_L=g192 R0_S=g210 R0_L=g210
Anti:   %R1_L<def> = LSRLrr %R1_L<kill>, %R0_S, pred:8, pred:%noreg
 Def Groups: R1_L=g210->g210(via R0_D)
 Antidep reg: R1_L Use Groups: R1_L=g210 R0_S=g210
Anti:   %R0_L<def> = TRLi9 16, pred:8, pred:%noreg
 Def Groups: R0_L=g210->g210(via R0_D)->g210(via R0_D)
 Antidep reg: R0_L
 Rename Candidates for Group g210:
  R0_D: elcInt64Regs :: R0_D R1_D R2_D R3_D R4_D R5_D R8_D R9_D R10_D R11_D
R12_D R13_D R14_D R15_D R16_D R17_D R18_D R19_D R20_D R21_D R22_D R23_D
R24_D R25_D
  R0_L: elcIntRegs elcIntAIRegs elcIntRegs elcIntRegs elcIntRegs elcIntRegs
:: R0_L R1_L R2_L R3_L R4_L R5_L R8_L R9_L R10_L R11_L R12_L R13_L R14_L
R15_L R16_L R17_L R18_L R19_L R20_L R21_L R22_L R23_L R24_L R25_L
  R1_L: elcIntRegs elcIntRegs elcIntRegs elcIntRegs elcIntRegs :: R0_L R1_L
R2_L R3_L R4_L R5_L R8_L R9_L R10_L R11_L R12_L R13_L R14_L R15_L R16_L
R17_L R18_L R19_L R20_L R21_L R22_L R23_L R24_L R25_L
  R0_S: elcShrtRegs elcShrtRegs :: R0_S R1_S R2_S R3_S R4_S R5_S R8_S R9_S
R10_S R11_S R12_S R13_S R14_S R15_S R16_S R17_S R18_S R19_S R20_S R21_S
R22_S R23_S R24_S R25_S
 Find Registers: [R12_D: R12_D R12_L R13_L R12_S]
 Breaking anti-dependence edge on R0_L: R0_D->R12_D(1 refs) R0_L->R12_L(7
refs) R1_L->R13_L(5 refs) R0_S->R12_S(2 refs)
 Use Groups:

 %R12_L<def> = TRLi9 16, pred:8, pred:%noreg
 %R13_L<def> = LSRLrr %R13_L<kill>, %R12_S, pred:8, pred:%noreg
 %R12_L<def> = LSRLrr %R2_L<kill>, %R12_S<kill>, pred:8, pred:%noreg,
%R12_L<imp-use,kill>
 %R13_L<def> = ANDLri %R13_L<kill>, 2047, pred:8, pred:%noreg
 %R12_L<def> = ANDLri %R12_L<kill>, 2047, pred:8, pred:%noreg
 %R4_D<def> = ASRDrr %R12_D, %R6_S, %R12_L<imp-def>, %R12_S<imp-def>,
%R13_S<imp-def>

On Mon, Jan 26, 2015 at 7:14 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> Daniil,
>
> Does the attached patch fix the problem?
>
> The problem is that we both keep the live super-register defined through
> the sub-register definitions so that those definitions on which the super
> register really depends will get renamed with it, and we also discard
> sub-register use locations as the sub-registers are redefined. The result
> is what you've seen: we'll rename the super register along with some, but
> not all, subregister definitions.
>
> As shown by your example, this is also somewhat unfortunate, because there
> is actually no need to rename the super register in this case (it is fully
> covered by later subregister definitions), but we don't seem to track
> enough information here to exploit that either.
>
>  -Hal
>
> ----- Original Message -----
> > From: "Daniil Troshkov" <troshkovdanil at gmail.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> > Sent: Monday, January 26, 2015 1:33:10 AM
> > Subject: Re: Fix the aggressive anti-dep breaker's subregister
> definition handling
> >
> >
> >
> > Thanks for reply. I found that my patch is not work so if you know
> > how to fix problem, please help me.
> > The description of problem below.
> > Thank you!
> >
> >
> > On Sun, Jan 25, 2015 at 12:08 AM, Hal Finkel < hfinkel at anl.gov >
> > wrote:
> >
> >
> > [moving patch review to llvm-commits]
> >
> > Hi Daniil,
> >
> > Thanks for the patch, unfortunately, I don't quite understand it...
> >
> > In your example we have:
> >
> > > def1(r0.32)
> > > def2(r1.32)
> > > def3(r0.32)
> > > use(r0.64)
> >
> > So, when examining use(r0.64) we should mark r0.64 as live, and
> > continue iterating backward. For each of def1,def2,def3, we should
> > see a def with a live super-register, and not mark the
> > super-register itself as defined at any of those locations. As a
> > result, the previous loop in PrescanInstruction over the
> > instruction's defined operands should have unioned the register
> > group of the defined operand with that of the live super-register.
> > As a result, all of those operands should have been renamed
> > together. Why is that not happening?
> >
> >
> > use(r0.64) - is live
> > def3(r0.32) - is not live
> > def2(r1.32) - is not live
> > def1(r0.32) - r0.64 is unioned because it is live, r0.32 and r1.32
> > are not unioned because they are not live.
> >
> >
> >
> >
> > It might be helpful for you to include the relevant parts of the
> > debug output from your test case, as I did in the commit message for
> > r202294, so that we can see directly what is going on.
> >
> >
> > R0_D = {R0_L, R1_L}
> > R0_L = {R0_S, R1_S}
> >
> > %R0_L<def> = TRLi9 16, pred:8, pred:%noreg
> > %R1_L<def> = LSRLrr %R1_L<kill>, %R0_S, pred:8, pred:%noreg
> > %R0_L<def> = LSRLrr %R2_L, %R0_S, pred:8, pred:%noreg,
> > %R0_L<imp-use,kill>
> > %R1_L<def> = ANDLri %R1_L<kill>, 2047, pred:8, pred:%noreg
> > %R0_L<def> = ANDLri %R0_L<kill>, 2047, pred:8, pred:%noreg
> > %R4_D<def> = ASRDrr %R0_D<kill>, %R6_S
> >
> >
> > Anti: %R4_D<def> = ASRDrr %R0_D<kill>, %R6_S
> > Def Groups: R4_D=g213->g215(via R4_S)->g214(via R4_L)->g216(via
> > R5_S)->g216(via R4_L)->g217(via R5_L)
> > Use Groups: R0_D=g0->g218(last-use) R1_L->g219(last-use)
> > R6_S=g204->g220(last-use)
> > Anti: %R0_L<def> = ANDLri %R0_L<kill>, 2047, pred:8, pred:%noreg
> > Def Groups: R0_L=g208->g209(via R0_S)->g218(via R0_D)->g210(via
> > R1_S)->g210(via R0_D)
> > Antidep reg: R0_L (real dependency)
> > Use Groups: R0_L=g210->g224(last-use) R0_S->g225(last-use)
> > R1_S->g226(last-use)
> > Anti: %R1_L<def> = ANDLri %R1_L<kill>, 2047, pred:8, pred:%noreg
> > Def Groups: R1_L=g219->g210(via R0_D)
> > Antidep reg: R1_L (real dependency)
> > Use Groups: R1_L=g210->g229(last-use)
> > Anti: %R0_L<def> = LSRLrr %R2_L, %R0_S, pred:8, pred:%noreg,
> > %R0_L<imp-use,kill>
> > Def Groups: R0_L=g224->g225(via R0_S)->g210(via R0_D)->g226(via
> > R1_S)->g226(via R0_D)
> > Antidep reg: R0_L Use Groups: R2_L=g192 R0_S=g226->g230(last-use)
> > R0_L=g226->g231(last-use) R1_S->g232(last-use)
> > Anti: %R1_L<def> = LSRLrr %R1_L<kill>, %R0_S, pred:8, pred:%noreg
> > Def Groups: R1_L=g229->g226(via R0_D)
> > Antidep reg: R1_L Use Groups: R1_L=g226->g233(last-use) R0_S=g230
> > Anti: %R0_L<def> = TRLi9 16, pred:8, pred:%noreg
> > Def Groups: R0_L=g231->g230(via R0_S)->g226(via R0_D)->g232(via
> > R1_S)->g232(via R0_D)
> > Antidep reg: R0_L
> > Rename Candidates for Group g232:
> > R0_D: elcInt64Regs :: R0_D R1_D R2_D R3_D R4_D R5_D R8_D R9_D R10_D
> > R11_D R12_D R13_D R14_D R15_D R16_D R17_D R18_D R19_D R20_D R21_D
> > R22_D R23_D R24_D R25_D
> > R0_L: elcIntRegs :: R0_L R1_L R2_L R3_L R4_L R5_L R8_L R9_L R10_L
> > R11_L R12_L R13_L R14_L R15_L R16_L R17_L R18_L R19_L R20_L R21_L
> > R22_L R23_L R24_L R25_L
> > R0_S: elcShrtRegs elcShrtRegs :: R0_S R1_S R2_S R3_S R4_S R5_S R8_S
> > R9_S R10_S R11_S R12_S R13_S R14_S R15_S R16_S R17_S R18_S R19_S
> > R20_S R21_S R22_S R23_S R24_S R25_S
> > Find Registers: [R12_D: R12_D R12_L R12_S]
> > Breaking anti-dependence edge on R0_L: R0_D->R12_D(1 refs)
> > R0_L->R12_L(2 refs) R0_S->R12_S(2 refs)
> > Use Groups:
> >
> >
> > %R12_L<def> = TRLi9 16, pred:8, pred:%noreg
> > %R1_L<def> = LSRLrr %R1_L<kill>, %R12_S, pred:8, pred:%noreg
> > %R0_L<def> = LSRLrr %R2_L<kill>, %R12_S, pred:8, pred:%noreg,
> > %R12_L<imp-use>
> > %R1_L<def> = ANDLri %R1_L<kill>, 2047, pred:8, pred:%noreg
> > %R0_L<def> = ANDLri %R0_L<kill>, 2047, pred:8, pred:%noreg
> > %R4_D<def> = ASRDrr %R12_D<kill>, %R6_S
> >
> >
> >
> >
> >
> > -Hal
> >
> >
> >
> > ----- Original Message -----
> > > From: "Daniil Troshkov" < troshkovdanil at gmail.com >
> > > To: hfinkel at anl.gov
> > > Cc: "LLVM Developers Mailing List" < llvmdev at cs.uiuc.edu >
> > > Sent: Saturday, January 24, 2015 11:17:10 AM
> > > Subject: Re: Fix the aggressive anti-dep breaker's subregister
> > > definition handling
> > >
> > >
> > >
> > > Oops! I'm sorry. Misprint:
> > >
> > >
> > >
> > > KillIndices[*SubR] > KillIndices[*AI]
> > >
> > >
> > > On Sat, Jan 24, 2015 at 8:05 PM, Daniil Troshkov <
> > > troshkovdanil at gmail.com > wrote:
> > >
> > >
> > >
> > >
> > >
> > > Hello Hal!
> > >
> > > r 202294
> > > Fix the aggressive anti-dep breaker's subregister definition
> > > handling
> > >
> > > There is a problem.
> > >
> > > For example:
> > > r0.64 = {r0.32, r1.32}
> > > r2.64 = {r2.32, r3.32)
> > >
> > > def1(r0.32)
> > > def2(r1.32)
> > > def3(r0.32)
> > > use(r0.64)
> > >
> > > Try to rename def1(r0.32). According current algo we get smth like:
> > >
> > > def1(r2.32)
> > > def2(r1.32)
> > > def3(r0.32)
> > > use(r2.64)
> > >
> > > Because r0.64 was still not defined.
> > >
> > > Patch to fix it:
> > >
> > > Index: lib/CodeGen/AggressiveAntiDepBreaker.cpp
> > > ===================================================================
> > > --- lib/CodeGen/AggressiveAntiDepBreaker.cpp (revision 227005)
> > > +++ lib/CodeGen/AggressiveAntiDepBreaker.cpp (working copy)
> > > @@ -326,6 +326,7 @@
> > > void AggressiveAntiDepBreaker::PrescanInstruction(MachineInstr *MI,
> > > unsigned Count,
> > > std::set<unsigned>& PassthruRegs) {
> > > + std::vector<unsigned> &KillIndices = State->GetKillIndices();
> > > std::vector<unsigned> &DefIndices = State->GetDefIndices();
> > > std::multimap<unsigned, AggressiveAntiDepState::RegisterReference>&
> > > RegRefs = State->GetRegRefs();
> > > @@ -396,7 +397,7 @@
> > >
> > > // Update def for Reg and aliases.
> > > for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
> > > - // We need to be careful here not to define already-live super
> > > registers.
> > > + // We need to be careful here to define already-live super
> > > registers.
> > > // If the super register is already live, then this definition is
> > > not
> > > // a definition of the whole super register (just a partial
> > > insertion
> > > // into it). Earlier subregister definitions (which we've not yet
> > > visited
> > > @@ -403,7 +404,16 @@
> > > // because we're iterating bottom-up) need to be linked to the same
> > > group
> > > // as this definition.
> > > if (TRI->isSuperRegister(Reg, *AI) && State->IsLive(*AI))
> > > - continue;
> > > + for (MCSubRegIterator SubR(*AI, TRI, false); SubR.isValid();
> > > ++SubR)
> > > + if (!TRI->isSubRegister(*SubR, Reg) &&
> > > + // We get only not defined *SubR.
> > > + // If *SubR was defined then *AI is not live but it is.
> > > + // So we can use IsLive check for *SubR.
> > > + (!State->IsLive(*SubR) ||
> > > + KillIndices[*SubR] < KillIndices[*AI])) {
> > > + KillIndices[*SubR] = KillIndices[*AI];
> > > + DefIndices[*SubR] = DefIndices[*AI];
> > > + }
> > >
> > > DefIndices[*AI] = Count;
> > > }
> > >
> > >
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150127/82a75e7f/attachment.html>