[PATCH] D32563: Improve code placement algorithm in Reassociate pass.

[Dehao Chen via llvm-commits]

On Thu, Apr 27, 2017 at 4:14 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>
>
> On Thu, Apr 27, 2017 at 4:02 PM, Xinliang David Li <davidxl at google.com>
> wrote:
>>
>>
>>
>> On Thu, Apr 27, 2017 at 2:38 PM, Daniel Berlin <dberlin at dberlin.org>
>> wrote:
>>>
>>>
>>>
>>> On Thu, Apr 27, 2017 at 1:42 PM, Wei Mi via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>>
>>>> On Thu, Apr 27, 2017 at 1:26 PM, Daniel Berlin via Phabricator
>>>> <reviews at reviews.llvm.org> wrote:
>>>> > dberlin added a comment.
>>>> >
>>>> > Isn't this close to what n-ary reassociate does?
>>>> > (if so, why not just use that and disable this?)
>>>> >
>>>>
>>>> Nary reassociate tries to maximize the redundency exposed by
>>>> reassociation.
>>>
>>>
>>> As does reassociate :)
>>>
>>>>
>>>> This patch tries to reduce the register pressure by
>>>> changing where to insert the intermediate results of reassociation,
>>>> and it doesn't try to improve the result of reassociation for better
>>>> CSE/PRE/LICM optimizations.
>>>
>>>
>>> I understand.
>>>
>>>>
>>>> The two have different motivations, so I
>>>> don't see how nary reassociation can be helpful here.
>>>
>>>
>>> The placement algorithm for computations that n-ary reassociate uses
>>> tries to reuse existing computation points.
>>> This patch tries to fix  "too many overlapped life-ranges. "
>>>
>>> The algorithm it uses, to find nearest common dominator of operands, etc,
>>> is pretty much what n-ary reassociate does:
>>> "
>>>   // Look for the closest dominator LHS of I that computes LHSExpr, and
>>> replace
>>>   // I with LHS op RHS.
>>> "
>>>
>>
>>
>> It seems to me that this may not necessarily result in reduced overlapping
>> live ranges -- it  can increase the live range of the temporary that holds
>> the value of LHS.
>
> Sometimes, yes.
> But you could also fix this.
>>
>> What this patch, on the other hand, can almost always result in
>> non-increased register pressure.
>
>
> Maybe?
>
> Remember that this reassociate has *zero* global view of expressions.
> It only makes locally good choices.
>
> IE given:
>      bar(a + b);
>      bar((a + 2) + b);
>
>
> This reassociate will process the a + b and a + 2 + b separately,

This patch will not change the original handling of this case because
both a and b has >1 uses.

>
> You are assuming the local choices you make about placement in such case
> turn out globally good, or even globally better.
>
> You already know i can prove this untrue :)
>