[PATCH] D151943: [InstCombine] Propagate some func/arg/ret attributes from caller to callsite

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D151943#4394217 <https://reviews.llvm.org/D151943#4394217>, @nikic wrote:

> In D151943#4394139 <https://reviews.llvm.org/D151943#4394139>, @goldstein.w.n wrote:
>
>> At the moment FunctionAttrs doesn't do any callsite modifications, only the functions themselves. If you think it fits, however, I'd be okay extending FunctionAttrs. Same for nocapture patch? Also should I wait on D152081 <https://reviews.llvm.org/D152081> or since thats too the attributor still makes sense to try and get something into FunctionAttrs?
>
> I think it's pretty unlikely that the Attributor will be enabled by default anytime soon, so I wouldn't wait on that. I expect a substantial investment in improving Attributor compile-times will be needed to make this even remotely viable.
>
> In D151943#4394179 <https://reviews.llvm.org/D151943#4394179>, @goldstein.w.n wrote:
>
>> Also I think then FunctionAttrs need to be moved to run before inlining:
>>
>>   ./bin/opt -O3 -print-pipeline-passes
>>   ...cgscc(devirt<4>(inline<only-mandatory>,inline,function-attrs<skip-non-recursive>...
>
> Unless you care about non-trivial SCCs, it does run before inlining due to CGSCC interleaving.
>
> An alternative to FunctionAttrs is doing this directly in InlineFunction -- in fact, we already transfer return attributes there in https://github.com/llvm/llvm-project/blob/36f351098cd50809658493d9b2e22a795874bab0/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1366. This could be extended to other attribute kinds. The benefit of doing it during inlining is that it's possible to transfer attributes from the call-site, rather than just the definition, though I'm not sure this is relevant for the cases you're interested in. You want this for the case where there are frontend-generated attributes on the definition, right?

The motivation is for me was the loss of user-specified attributes, but could imagine other cases this is helpful.
How does making it an analysis function (not sure which file), something like "getPropagatedAttributesToCallsite(ArgAttrsOut, FuncAttrsOut, RetAttrsOut)" so it can be used in inline/functionAttrs.
My only concern with doing inline only is I think something is 'isSpeculativelyExecutable' when running over a readnone function will miss that the callsite is now implied readnone (assuming no allocas) which could also potentially be useful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151943/new/

https://reviews.llvm.org/D151943