balazs-benics-sonarsource wrote: > However, I think I see another, less complex approach for "fixing" the method > `acquire`: instead of creating the `Dummy` symbol and calculating its > complexity early, just modify the original state of the method (i.e. the > parent revision of > [8c29bbc](https://github.com/llvm/llvm-project/commit/8c29bbc6c309efe0a1d6af948f95dbb9654971b8) > ) by moving the block > > ``` > if (SD->complexity() > MaxCompComplexity) { > return cast<Ret>(acquire<SymbolOverlyComplex>(SD)); > } > ``` > > out of the `if (!SD)` block (placing it directly after the `if (!SD)` block). > > This would guarantee that: > > * we always return `SymbolOverlyComplex` if the expression is too complex > (even if there is a cache hit); > * but even these overly complex symbols (which only live "inside" the > `SymbolOverlyComplex` block) enjoy the benefit of caching and aren't > recreated repeatedly.
You are right. Indeed this is a much simpler solution, IDK how I overlooked it xD > I hope that this alternative approach could prevent the performance hit that > you observed. No, it's on par. And now as I'm thinking about this a bit more it makes sense. Here is the flame graph that I get no matter what I do with this patch. It will make sense.  We see a lot of `simplifySValOnce` that decomposes the symbol and traverses the tree. With my original patch it's not present likely because the `Unknowns` poison the the tree so there is nothing to be traversed. This is why with my original patch we actually improve the performance of the sample rather than pessimising it. How I look at this is that returning `Unknown` is the right thing to do. What I do not yet understand how the baseline does not have these `simplifySValOnce` traversals in its flamegraph, but I'm not sure it's important if the observable report differences with the solution that returns `Unknown` is practically none. https://github.com/llvm/llvm-project/pull/144327 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
