[PATCH] D17598: Use the same attributes as "memset" when introducing a call to memset_pattern in LoopIdiom

[Ahmed Bougacha via llvm-commits]

On Thu, Feb 25, 2016 at 6:01 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>
>> On Feb 25, 2016, at 1:42 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
>>
>> ab added a subscriber: ab.
>> ab added a comment.
>>
>> This is IMO a little too hacky;
>
> I can see it as hacky (because LLVM does not provide better infrastructure), but *too* hacky I don't know ;)
>
>> what do you think of exposing inferattrs into a utility function?
>
> That sounds hacky to me as well :(

My main concern is with the idea of mixing attributes for different
functions. In practice, it probably will never be a problem, but
still.

> I wouldn't want to create a raw call and then having to run to a utility that magically infer something on the call, while the knowledge of the semantic of the call is there.
> We don't do it for intrinsic like memset, why do we have to do it for libfunc?
>
> I could imagine one "good" solution to have a better handling of library calls, where these would be somehow modeled in tablegen (like we do for intrinsics), and client would go through this unique interface to interact with lib calls. The attributes (and other properties would be managed in a single place).
>
> This seems like a larger refactoring / infrastructure work that is interesting, but that I see as beyond the scope of this small fix: manually adding the attributes is both not a pervasive change and helps on benchmarks (we have a benchmark we where LTO is significantly better than O3 because of this issue).

Without going that far, TLI does seem like this single place to
interact with libcalls.

How about moving the logic out of InferFunctionAttrs, into TLI? The
pass would then be a tiny wrapper around TLI.
Also, since the logic checks that the prototypes are correct, that's a
good first step towards putting that information in TLI, which has
long been a plan of mine.

-Ahmed

> --
> Mehdi
>
>
>>
>>
>> http://reviews.llvm.org/D17598
>>
>>
>>
>