[PATCH] D30759: With PIE on x86_64, keep hot local arrays on the stack

[Wei Mi via llvm-commits]

On Tue, Mar 14, 2017 at 3:29 PM, Sriraman Tallam <tmsriram at google.com> wrote:
> On Mon, Mar 13, 2017 at 5:20 PM, Wei Mi <wmi at google.com> wrote:
>> On Wed, Mar 8, 2017 at 4:31 PM, Eli Friedman via Phabricator via
>> llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>> efriedma added a comment.
>>>
>>>>> In theory, the C/C++ standards require behavior equivalent to -fno-merge-all-constants.  In practice, code doesn't actually depend on that, so we made the decision many years ago to turn on -fmerge-all-constants by default.
>>>>
>>>> Understood.  Does it seem reasonable/useful to fix this along the
>>>>  lines of GCC, -fmerge-constants and -fmerge-all-constants where the
>>>>  latter applies to const arrays and a warning that this is happening
>>>>  when the latter option is used?
>>>
>>> -fmerge-all-constants has exactly the same meaning in clang and gcc.  And it's generally beneficial for both codesize and performance, so turning it off to pursue performance is a bad idea.
>>>
>>
>> I think we will keep the existing benefit of -fmerge-all-constants for
>> codesize and performance. We just don't want -fmerge-all-constants to
>> do the merge for "ALL" constants, because we want to keep the array on
>> stack for limited beneficial cases like the one in the original post.
>> -fmerge-all-constants currently blocks such opportunity.
>>
>>> I would suggest finding some other approach to solve your issue later in the optimization pipeline, preferably in a manner which is sensitive to register pressure.
>>
>> The condition to do the transformation is that the place to use the
>> array is hotter than where it is allocated. It seems enough to
>> guarantee the benefit, and it avoids counting on register pressure
>> which is more difficult to precisely evaluate in middle end.
>>
>>>  Maybe put the code in ConstantHoisting?
>>
>> Yes, a pass late in the pipeline is a good idea. But I am not sure
>> ConstantHoisting is quite fitted since ConstantInt is the target
>> object there while constant array is the target object here. The
>> transformation is also different.
>>
>> I didn't find a perfect pass for it. Maybe a new pass is not a too bad idea?
>
> Thanks for the comments Wei. Just want to also add that Eli's point is
> that register pressure is really when you need this optimization.
> Without that, the lea can be hoisted out of the loop and it should not
> affect performance as it is done only once on entry.
>

I missed that point. Thanks for explaining it. I agree with it.

It seems it is the time to write some utility or an analysis pass so
it is easy to query the register pressure for certain scope on llvm IR
level. LSR wants it for a long time. The optimization here increases
its importance. This is something Quentin mentioned in his todo list.
If he is too busy to do that recently, I can give it a try after I
finish the bitfield shrinking stuff.

To avoid the work from being blocked, can we guard the optimization
(without register pressure evaluation) by an option and let it stay
off by default. Once the register pressure evaluation facility is
available, we can turn it on.

Thanks,
Wei.

>>
>>> You don't lose any useful information by promoting the alloca to a global constant; you can easily recreate it later.
>
> True, this would also mean that this optimization would convert a
> previously defined global array to a stack array for some functions,
> more generic.
>
>>
>> Good point. It means we can turn global constant array access to local
>> array access in certain scope by initializing a newly created local
>> array with the content of the global constant array, if only the
>> accesses in the scope are hotter enough than the initialization. This
>> is more general than just keeping local constant array as local when
>> it is beneficial.
>>
>> Thanks,
>> Wei.
>>
>>>
>>>
>>> https://reviews.llvm.org/D30759
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits