[PATCH] Fix a performance problem in gep(gep ...) merging

[Daniel Berlin]

On Tue, Apr 14, 2015 at 9:41 AM Quentin Colombet <qcolombet at apple.com>
wrote:

> On Apr 14, 2015, at 8:46 AM, Daniel Berlin <dberlin at dberlin.org> wrote:
>
> I think nick's question is more "is GEP merging really the only problem
> this causes?"
>
> I know the answer is no, because PRE will go crazy and do this if you make
> it powerful enough to see loop carried dependences and in GCC, it uses an
> API to detect whether an insertion would be causing issues like this and
> avoids doing it (which I believe a exactly Nick's suggestion - create a
> utility others can use to avoid accidentally doing transforms like this in
> the first place)
>
>
> Although I agree to the general goal, I think it is difficult to derive a
> good abstraction from just one use case.
>

Fair enough


> I suggest we go for this simple fix to get the immediate performance gain
> and think for a generalize abstraction when we see actual motivating
> examples.
>

I have no problem with this, as long as we actually do that :)


>
> Thoughts?
>
> -Quentin
>
>
>
>
> On Tue, Apr 14, 2015 at 8:26 AM Xinliang David Li <xinliangli at gmail.com>
> wrote:
>
>> IIUC, GEP merging can be harmful regardless of where the target is in
>> loop context or not. Wei's heuristic filters out lots of harmful cases. A
>> more general fix might need introduce more sophisticated cost analysis --
>> it is possible (as mentioned by Quentin) for this simple heuristic to
>> filter out beneficial merging, but test cases are needed for that.
>>
>> David
>>
>> On Tue, Apr 14, 2015 at 1:16 AM, Nick Lewycky <nicholas at mxc.ca> wrote:
>>
>>> Quentin Colombet wrote:
>>>
>>>> The thing with the "both constants" approach is that we may miss some
>>>> cases where an addressing mode may apply. In particular, on x86, when one
>>>> argument of the add is a constant, we would be able to match reg1 + reg2 +
>>>> displacement.
>>>>
>>>> That being said, your numbers imply this is not a problem in practice,
>>>> and most targets do not support reg1 + reg2 + reg3/imm addressing mode.
>>>> Anyway, we could recover from that later on if needed.
>>>>
>>>> So LGTM.
>>>>
>>>
>>> I might be misunderstanding why this patch helps, please bear with me.
>>> Is this not a special case of a general problem, mainly that we're merging
>>> something into a loop that was previously two operations one inside and one
>>> outside the loop? Why wouldn't this problem come up in other ways? We
>>> already have other problems where we introduce loop carried dependencies
>>> where ones previously didn't exist, shouldn't we approach this by adding an
>>> API to detect those and then making transforms conditional on that?
>>>
>>> Nick
>>>
>>>
>>>
>>>> Thanks Wei!
>>>> -Quentin
>>>>
>>>>
>>>> REPOSITORY
>>>>    rL LLVM
>>>>
>>>> http://reviews.llvm.org/D8911
>>>>
>>>> EMAIL PREFERENCES
>>>>    http://reviews.llvm.org/settings/panel/emailpreferences/
>>>>
>>>>
>>>>
>>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150414/2ec74d52/attachment.html>