[PATCH] D156397: [FunctionAttrs] Unconditionally perform argument attribute inference in the first function-attrs pass

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

In D156397#4570516 <https://reviews.llvm.org/D156397#4570516>, @cfang wrote:

> In D156397#4570449 <https://reviews.llvm.org/D156397#4570449>, @aeubanks wrote:
>
>> last nit then I think this is good to go
>> `function-attrs<skip-non-recursive>` should be renamed to something like `function-attrs<non-recursive-only-arg-attrs>`
>
> Should it be function-attrs<recursive-or-arg-attrs-only> ?

The "or" is very declarative as opposed to imperative, which I think is more confusing. I imagine `function-attrs` as performing everything, then `<foo>` is a constraint on top of that, so something like `<skip-non-recursive-function-attrs>` makes more sense to me.



================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:1769
                                                   CGSCCUpdateResult &) {
   // Skip non-recursive functions if requested.
+  // Only infer readonly etc on arguments in that case, because it can affect
----------------
I'd remove this comment and change below to `Only infer argument attributes for non-recursive functions, because...`


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

https://reviews.llvm.org/D156397