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

[Mehdi Amini via llvm-commits]

> On Feb 26, 2016, at 2:59 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
> 
> 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.

Do you mean using the attributes from memset on memset_pattern?
I wanted to avoid adding a static list of attribute for maintenance reason (the day ArgMemOnly was created and added, you would have to audit all the optimizer for places that create a memset_pattern). 
Arguably, I can conceive that some attributes not-yet-created wouldn't apply to both and then bad-thing-would-happen.


> 
>> 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 lib calls.

Good point, there is this weirdness with intrinsics that makes it not always clear.

> How about moving the logic out of InferFunctionAttrs, into TLI? The
> pass would then be a tiny wrapper around TLI.

I like this refactoring!

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

Looks like we have a plan then :)

-- 
Mehdi