wenlei marked 4 inline comments as done. wenlei added inline comments.
================ Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:272 + else + VectLibrary = Impl.ClVectorLibrary; + ---------------- tejohnson wrote: > Suggest moving the implementation of this constructor to the .cpp file, in > which case you can just set VectLibrary directly from ClVectorLibrary there > and remove the member on the Impl object. There're utilities that use `TargetLibraryInfo`, but don't link with `TargetLibraryInfo.o`. And looking at `TargetLibraryInfo`, all of the functions are in this header, so I assumed it's intentional to keep this type self-contained in this header, as it's public API, which is why I add `ClVectorLibrary` to Impl to pass it back to `TargetLibraryInfo`. For `TargetLibraryInfoImpl`, it's ok to have the implementation outside of the header. I can give it a try if keeping the class implementation/definition self-contained in the header isn't important. ================ Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:577 memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray)); - VectorDescs = TLI.VectorDescs; - ScalarDescs = TLI.ScalarDescs; + for (unsigned i = 0; i < sizeof(TLI.VecLibDescs) / sizeof(TLI.VecLibDescs[0]); + i++) ---------------- tejohnson wrote: > Why not just have "i < NumVecLibs"? Good catch, thanks.. 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
