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

[Hal Finkel]

[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?

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.

 -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