[PATCH] D94585: [IndirectFunctions] Skip propagating attributes to address taken functions

[Konstantin Zhuravlyov via Phabricator via llvm-commits]

kzhuravl added a comment.

In D94585#2524476 <https://reviews.llvm.org/D94585#2524476>, @madhur13490 wrote:

> In D94585#2520966 <https://reviews.llvm.org/D94585#2520966>, @kzhuravl wrote:
>
>> This change is causing infinite loop when compiling rocThrust and hipCUB. Can you take a look? Thanks.
>
> FWIW, after testing rocThrust and hipCUB, it turned out that, **it is not an infinite loop** but a rise in compile-time which crossed timing threshold of the internal infra. The apps eventually compiled with 1-2% increment in the compile-time. I figured out the cause behind this. This patch makes two additional calls to Function::hasAddressTaken() and hasAddressTaken()   is not optimal. Each time  Function::hasAddressTaken() is called, it traverses over all uses of the function to deduce the required info. The calls made by this patch are itself in the loop which effectively made the suboptimality of  Function::hasAddressTaken() conspicuous. In the new patch I am going to remove one call to  Function::hasAddressTaken() which still preserves the intention of this patch. The new patch would basically be Diff3 of this patch.
>
> We should think about optimizing Function::hasAddressTaken() by introducing a bool in Function class which would cache the result. In addition to this, Function::hasAddressTaken() can accept an additional bool parameter and based on its truthness, the function can return the cached value. The default value of parameter bool should be false to preserve today's behavior but client can set it to "true" if they want the cached value. Latter would be useful for this patch as the information is unlikely to change in a module (which is static naturally). This optimization would significantly improve the run-time of hasAddressTaken().

Without this change, internal ci takes ~10 minutes on average to compile rocThrust, ~9 minutes on average to compile hipCUB.

With this change, internal ci timed out after compiling rocThrust for 1.5 hours, timed out after compiling hipCUB after 1 hour.

When bisecting on my local machine, I gave 1 hour on top of what ci gave before terminating...

It did *not* look like a 1-2% increase in time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94585