[PATCH] D22778: Add Loop Sink pass to reverse the LICM based of basic block frequency.

[Xinliang David Li via llvm-commits]

On Thu, Aug 4, 2016 at 4:53 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
> (sorry for not replying immediately)
>
> On Sun, Jul 31, 2016 at 7:31 AM Dehao Chen via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Sorry I didn't update the original thread before I implement this patch.
>> My understanding was Hal proposed IR level sinking at CGP, and I proposed
>> enhancing MachineSinking and Hal thinks that's also an workable plan.
>>
>> But when I started implementing this in MachineSinking, I found that the
>> alias info support for loop at machine level is too weak. That's why
>> MachineLICM only handles function invariant load motion instead of loop
>> invariant load motion. I guess that's also the reasoning of the original
>> comment in MachineSink.cpp (Machine sinking should not replace IR sinking).
>
>
> Ah, thanks. This is a great bit of context. It'd be good to mention it in
> the patch description
>
>>
>>
>> For now, the design is:
>>
>> 1. IR LICM handles most accurate hoisting at early stage
>> 2. IR Sinking handles most accurate sinking before CGP
>> 3. Machine LICM handles simple hoisting at machine level
>> 4. MachineSinking handles simple sinking at machine level
>>
>> I think this design makes sense because:
>>
>> 1. We don't need to introduce sophisticated alias analysis to machine
>> level loops
>> 2. It's in perfect symmetry
>>
>> But I'm open to discussions on other design alternatives.
>
>
> This design looks great to me, especially considering the interesting
> discovery you made regarding alias info. Of course, it be good to make sure
> it mkaes sense Hal as well since he was in the original discussion, but I
> can't imagine he'd have big objections....
>
> Two things that jump out at me at a high level:
>
> - Should this be a fully general IR sinking pass rather than just a *loop*
> sinking pass? I think it'd be fine even if all the non-loop logic was just
> documented as an area for future work. If it makes sense in general, you
> could leave the framework in place for others to flesh out as test cases and
> such come up.

I forgot to reply to this. I think longer term, this should become a
general IR level sinking pass to probably replace IR sinking (cmp
sinking, address computation sinking etc) done in CGP as well.

David
>
> - It might be worth adding comments to this and the other passes you mention
> that document this nice symmetric design and the rationale. Your explanation
> here is really great, I just want to make sure others can find it later! =D
>
> -Chandler