[PATCH] D97735: [Globals] Treat nobuiltin fns as maybe-derefined.

[Zaara Syeda via Phabricator via llvm-commits]

syzaara added a comment.

ping
Hi @Florian, could you please provide your thoughts on the earlier comment?

In D97735#3784324 <https://reviews.llvm.org/D97735#3784324>, @syzaara wrote:

> @Florian 
> Hello. I am uncertain whether having ‘nobuiltin’ attribute on a function declaration should imply that it returns true for mayBeRedefined. The description for ‘nobuiltin’ in the llvm reference does not imply that the function definition may change. We should still be able to enable IPO transformations for functions with nobuiltin if the transformations do not make assumptions about the semantics of the function. There is also no statement that ‘nobuiltin’ applies only to library functions. If the attribute is used on non library user function, all IPO transformations are then blocked on this function. One suggested workaround could be to remove the ‘nobuiltin’ attribute from user functions that are not library calls in InferFunctionAttrs.cpp. with something like:
>
>   +++ b/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
>   @@ -21,7 +21,7 @@ static bool inferAllPrototypeAttributes(
>      Module &M, function_ref<TargetLibraryInfo &(Function &)> GetTLI) {
>     bool Changed = false;
>    
>   - for (Function &F : M.functions())
>   + for (Function &F : M.functions()) {
>      // We only infer things using the prototype and the name; we don't need
>      // definitions. This ensures libfuncs are annotated and also allows our
>      // CGSCC inference to avoid needing to duplicate the inference from other
>   @@ -32,7 +32,12 @@ static bool inferAllPrototypeAttributes(
>        Changed |= inferNonMandatoryLibFuncAttrs(F, GetTLI(F));
>       Changed |= inferAttributesFromOthers(F);
>      }
>   -
>   +  if (!F.isDeclaration()) {
>   +   LibFunc LibFn;
>   +   if (F.hasFnAttribute(Attribute::NoBuiltin) && !GetTLI(F).getLibFunc(F, LibFn))
>   +    F.removeFnAttr(Attribute::NoBuiltin);
>   +  }
>   + }
>     return Changed;
>    }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97735