tejohnson added a comment. In D77632#1976240 <https://reviews.llvm.org/D77632#1976240>, @wenlei wrote:
> In D77632#1974409 <https://reviews.llvm.org/D77632#1974409>, @tejohnson wrote: > > > In D77632#1974363 <https://reviews.llvm.org/D77632#1974363>, @wenlei wrote: > > > > > In D77632#1974015 <https://reviews.llvm.org/D77632#1974015>, @nikic wrote: > > > > > > > This change causes a ~0.5% compile-time regressions: > > > > http://llvm-compile-time-tracker.com/compare.php?from=5b18b6e9a84d985c0a907009fb71de7c1943bc88&to=60c642e74be6af86906d9f3d982728be7bd4329f&stat=instructions > > > > This is quite a lot as these things go, so it would be great if you > > > > could double check if there's any optimization potential here. In > > > > particular I'm wondering why this affects normal builds so much, even > > > > though they (presumably?) don't use any veclib at all. > > > > > > > > > Thanks for the heads-up. This is surprising but there is a change even > > > when veclib is not used - in order to allow each function to use > > > different veclib without duplicating the work of populating vector > > > function list for each function, we now always pre-populate vector > > > function list for three supported vector libraries for each module. > > > However 0.5% compile-time for that work given it's per-module is not > > > expected. I suspect we may be passing/copying TLII around more than we > > > anticipated (now we always have more stuff to copy). I will take a look. > > > We could also turn this into a lazy initialization - only populate the > > > needed list for module level TLII when it's first queried by a function > > > level TLI. > > > > > > Hmm, yeah that is surprising, because the TLII should be built once per > > module per TLI analysis, which is never invalidated. We've gone from > > populating one set of vec libs to 3, I wouldn't have thought that was > > particularly expensive, so it would be good to see what is going on here > > and confirm we are only building this once as expected. > > > > Looking at the compile time data at that link, interestingly the > > "instructions" metric increased, but not wall time or cycles or task clock > > - they were all neutral. > > > Turns out there're a few places where we call copy ctor for TLI unnecessarily. I assume you mean the TargetLibraryInfoImpl (TLII) here, not the TargetLibraryInfo (TLI), right? The latter should be cheap to copy. Are these the changes in BackendUtil.cpp in D77952 <https://reviews.llvm.org/D77952>? I had a question about that on that patch as I think we will be calling the initializer more. Mostly we should only be copying the TargetLibraryInfo during optimization though, and not the TLII impl object. > Made some changes in D77952 <https://reviews.llvm.org/D77952> to use move > when possible. In addition, I should have used move for `TLI.VecLibDescs` in > move ctor of `TargetLibraryInfoImpl` too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/ https://reviews.llvm.org/D77632 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
