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

[Madhur Amilkanthwar via Phabricator via llvm-commits]

madhur13490 added a comment.

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().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94585