https://github.com/SLTozer approved this pull request.
This looks like a nice performance improvement - there's a statistically-significant reduction in max-rss as well for some builds, which I suspect comes from inserting fewer DILocations into the global unique DILocation DenseMap. Previously I think that DILocations would be distinct iff they were inlined call locations, which could theoretically lead to changes in output, as this creates the potential for identical-but-different DILocations to be created. For example, if we have 3 DILocations in a function before it is inlined: `(line: 10, scope: !1)`, `(line: 10, column: 3, scope: !1)`, `(line: 10, column: 7, scope: !1)` Then we inline the function, resulting in: `distinct (line: 10, scope: !1, inlinedAt: !5)`, `distinct (line: 10, column: 3, scope: !1, inlinedAt: !5)`, `distinct (line: 10, column: 7, scope: !1, inlinedAt: !5)` Then, if we merge the last two locations, we'll end up with a unique DILocation which is equivalent to the first but not equal: `(line: 10, scope: !1, inlinedAt: !5)` However, I don't _think_ that in practice this results in any correctness issues (doesn't seem to be any change in `size-file` on the compile time tracker), and any performance loss it may cause is clearly smaller than the gain from the change. https://github.com/llvm/llvm-project/pull/204817 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
