wenlei added a comment. In D77632#1975619 <https://reviews.llvm.org/D77632#1975619>, @mehdi_amini wrote:
> The existing TLI provides a very convenient way to define a VecLib without > LLVM knowing about it ahead of time. This feature is important for any > embedded use of LLVM as a library out-of-tree (I'll add a unit-test in-tree). > I don't think it is a big change to this patch to preserve the current > ability but I wanted to check first (and in the meantime I reverted in > temporarily in https://reviews.llvm.org/D77925 to avoid the feature > regression). > > At the moment the place where you seem to use this knowledge is with the > `enum VectorLibrary` in the `TargetLibraryInfoImpl` class, and the > `VecLibDescs` array which statically contains the known VecLib. > It seems to me that if we replace this enum with a string instead to > identify the VecLib everything should still hold together and this would fit > with minor changes to this path. The `VecLibDescs` could just be a > `StringMap<VectorLibraryDescriptors>` in this case. > > That was a third-party (in my case the XLA compiler) can still register its > own "XLA" VecLib and add all the descriptors. > > How does it sound? Thanks for the explanation about the revert. The proposal of using a StringMap to maintain the openness sounds good to me. And agree with @tejohnson, if the openness is a feature, it should be covered in tests, otherwise it can feel somewhat like a loophole and prone to breakage, though I can see how it can be useful.. Hope this patch can be restored with tweaks soon (we have workloads with very visible vectorization that relies on this). 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
