[PATCH] [InstCombine] Lower unknown @llvm.objectsize early.

[Nick Lewycky]

Ahmed Bougacha wrote:
> On Fri, Jan 23, 2015 at 12:33 AM, Nick Lewycky<nicholas at mxc.ca>  wrote:
>> Ahmed Bougacha wrote:
>>>
>>> We already try to lower objectsize in InstCombine, but only if the
>>> result is known.  When it is unknown, the intrinsic calls would survive
>>> the mid-level optimizers, to be lowered late, in CodeGenPrepare.
>>>
>>> We can lower them in InstCombine as well, to 0 or -1, depending on the
>>> min argument.
>>> One could argue this is a bit early to do the lowering, since the size
>>> might be made apparent by later optimizations.
>>
>>
>> Yep.
>>
>>    In practice however,
>>>
>>> the majority of cases has a never-known objectsize, and in a lot of
>>> the remaining few, the size is immediately apparent (say from a
>>> global value, or an alloca).
>>
>>
>> Sure I'd expect that, but that isn't really a problem on its own.
>>
>>> Always keeping the intrinsic call intact prevents optimizations, and
>>> makes memcpyopt useless when libcall fortification is enabled (the
>>> default on a few major targets).
>>
>>
>> Can we fix that instead? Not waiting as long as possible to lower unknown
>> llvm.objectsize just can't be right.
>
> I think the right way to fix that would be to also turn fortified
> libcalls into intrinsics: if we want to start treating them like their
> non-checking counterparts, we should just represent them the same way.

I'm ambivalent on this point. The libcall intrinsics usually capture 
information that the usual call wouldn't, for example the memcpy 
intrinsic captures the alignment and pointer addresses. If the same is 
true for the fortify equivalents then it may make sense. If it doesn't, 
then we probably shouldn't.

> The objectsize, whether known or not, should really be something we
> can drop when it's convenient, much like arithmetic flags.

I'm not sure why that's true. It seems like ignoring the presence of an 
objectsize is really easy? Is there something that makes this 
particularly compile-time expensive?

> If it sounds reasonable, I'll tinker with that.
>
> For now, I'm curious: what do you think of the other alternatives
> below?  Say, lowering objectsize right before memcpyopt?
>
>>> There are a few alternatives to doing it here; all seem overkill:
>>> - teach the libcall simplifier to look through unknown
>>>     @llvm.objectsize, so it's able to replace the fortified version
>>>     of a libcall with the non-fortified one if the size is unknown.
>>> - make a dedicated pass that lowers the intrinsic, even when
>>>     the size is unknown, run it before memcpyopt&   friends.
>>> - in memcpyopt, start by running the libcall simplifier, to
>>>     lower @llvm.objectsize, even when the size is unknown.
>>
>>
>> The first of those sounds best to me.
>
> I should have made it explicit, but the libcall simplifier is run as
> part of InstCombine, so both approaches are equivalent (I also started
> out with the simplifier, in D5984.)

Oh I forgot about that.

Memcpyopt is an interesting one in specific, it should try to lower it. 
It would be a good idea to factor "try to solve this llvm.objectsize" 
out to a utility function for other passes to use, and then make 
memcpyopt use it. However, it shouldn't be deleted at this stage if left 
unsolved, I think that power should remain with codegenprep.

>
> -Ahmed
>
>>> Also, Nuno proposes a related improvement to the associated
>>> APIs (MemoryBuiltins.h): "[the analyzer] could be improved to
>>> produce an interval instead.  If we know that the minimum
>>> objectsize is larger than the written size, then the check could
>>> go away, even if we don't know the exact size of the buffer."
>>>
>>> http://reviews.llvm.org/D7129
>>>
>>> Files:
>>>     lib/Transforms/InstCombine/InstCombineCalls.cpp
>>>     test/Transforms/InstCombine/objsize.ll
>>>     test/Transforms/InstCombine/stpcpy_chk-1.ll
>>>     test/Transforms/InstCombine/strcpy_chk-1.ll
>>>
>>> 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
>>
>>
>