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

[Daniel Berlin]

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)

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150414/9dec9cfc/attachment.html>