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