tejohnson added a comment.

In D68028#1685912 <https://reviews.llvm.org/D68028#1685912>, @gchatelet wrote:

> @tejohnson I believe this is the missing part for D67923 
> <https://reviews.llvm.org/D67923>.


Thanks, yep I will take a closer look at the patch today.

> I'm unsure if we still need the `BitVector` at all in the `TLI` since it 
> could be a simple attribute lookup on the function.

If we didn't save the info on the TLI we would instead need to save the 
Function object in the TLI and query the attribute info off the Function on 
every lookup, which seems heavier weight. I think caching the info in that 
object for fast lookup is going to be better. And as noted in the comments 
there, we can replace it with the existing AvailableArray moved from the base 
Impl object into the TLI and remove the override bitvector once this goes in, 
we use these attributes to set the TLI info on construction, and we remove the 
clang code that sets the unavailability from the CodeGenOpts which will no 
longer be needed. If this patch goes in first I can just modify my TLI patch to 
do that all in one go. Maybe that is best...

> Do you see any problematic interactions with the inlining phase?

The inliner will need to be modified to respect the function attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to