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

[Ahmed Bougacha]

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.
The objectsize, whether known or not, should really be something we
can drop when it's convenient, much like arithmetic flags.
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.)

-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
>
>