<div dir="ltr"><div class="gmail_extra">Thanks for reply. I found that my patch is not work so if you know how to fix problem, please help me.</div><div class="gmail_extra">The description of problem below.</div><div class="gmail_extra">Thank you!<br></div><div class="gmail_quote"> </div><div class="gmail_quote">On Sun, Jan 25, 2015 at 12:08 AM, 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:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">[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 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?<br></blockquote><div> </div><div>use(r0.64) - is live</div><div>def3(r0.32) - is not live</div><div>def2(r1.32) - is not live</div><div>def1(r0.32) - r0.64 is unioned because it is live,  r0.32 and r1.32 are not unioned because they are not live.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
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.<br></blockquote><div> </div><div>R0_D = {R0_L, R1_L}</div><div>R0_L = {R0_S, R1_S} </div><div> </div><div>  %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, %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 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->g224(last-use) R0_S->g225(last-use) 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, %R0_L<imp-use,kill><br> Def Groups: R0_L=g224->g225(via R0_S)->g210(via R0_D)->g226(via R1_S)->g226(via R0_D)<br> 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)<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 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 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 :: 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 R12_S]<br> 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)<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, %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</div><div> <br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
 -Hal<br>
<div><div><br>
----- Original Message -----<br>
> From: "Daniil Troshkov" <<a href="mailto:troshkovdanil@gmail.com" target="_blank">troshkovdanil@gmail.com</a>><br>
> To: <a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a><br>
> Cc: "LLVM Developers Mailing List" <<a href="mailto:llvmdev@cs.uiuc.edu" target="_blank">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 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" target="_blank">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 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 not<br>
> // a definition of the whole super register (just a partial 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>
</div></div><span><font color="#888888">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><div class="gmail_extra"><br></div></div>