[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D61634#1502685 <https://reviews.llvm.org/D61634#1502685>, @gchatelet wrote:

> In D61634#1502201 <https://reviews.llvm.org/D61634#1502201>, @tejohnson wrote:
>
> > Using function level attributes instead of module flags does provide finer grained control and avoids the conservativeness when merging IR for LTO. The downsides I see, mostly just in terms of the engineering effort to get this to work, are:
> >
> > - need to prevent inlining with different attributes
>
>
> IIUC this is needed regardless of the proposed change. Correct?


With the module flags approach, no - because there won't be fine grained enough info to do so. Any merged IR will need to use the conservatively set merged flag for the whole module. Or did you mean in comparison with the approach in this patch? I haven't looked at this one in any detail yet.

> 
> 
>> - currently the TargetLibraryInfo is constructed on a per-module basis. Presumably it would instead need to be created per Function - this one in particular seems like it would require fairly extensive changes.
> 
> Yes this one is a bit worrying.
>  I think we can discard right away any solution that would mutate or create a TLI on a per function basis.
>  Another design would be something like the following:
> 
>   auto FunctionTLI = ModuleTLI.createCustomizedTLI(F);
> 
> 
> where `FunctionTLI` is itself a `TargetLibraryInfo` and calls to `FunctionTLI` would either return the function customizations or delegate to the module's TLI. WDYT?

I don't think this makes it much easier - all TLI users still need to be taught to get and use this new Function level TLI. I guess for Function (or lower) passes it would be fairly straightforward, but for things like Module level or SCC passes it will require more wiring to ensure that the FunctionTLI is accessed in the appropriate places. Doable just a lot of manual changes.

> I'm unsure if we want to support function level attribute right away or if it's OK to be in an intermediate state with only module level attributes.

I'm interested in thoughts from other developers here. The module flag change is straightforward, but unnecessary churn if we want to go the function attribute route. Which despite the work seems like the best long term approach...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634