[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

tejohnson added a comment. In D77632#1976308 , @mehdi_amini wrote: > In D77632#1976231 , @wenlei wrote: > > > And agree with @tejohnson, if the openness is a feature, it should be > > covered in tests, otherwise

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

nikic added a comment. I gave D77952 a try (on top of this one), but didn't see a significant improvement from that change. Looking at the callgrind output for compilation of a **small** file, I see 52M total instructions, 4 calls to TLII initialization, where

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

tejohnson added a comment. In D77632#1976240 , @wenlei wrote: > In D77632#1974409 , @tejohnson wrote: > > > In D77632#1974363 , @wenlei wrote: > > > > > In D77632#1974015

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

mehdi_amini added a comment. In D77632#1976231 , @wenlei wrote: > 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 The thing is that LLVM does

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei added a comment. In 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

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei added a comment. In D77632#1974409 , @tejohnson wrote: > In D77632#1974363 , @wenlei wrote: > > > In D77632#1974015 , @nikic wrote: > > > > > This change causes a

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

mehdi_amini added a comment. In D77632#1976126 , @tejohnson wrote: > I think this should work. Just reiterating something we chatted about off > patch yesterday, we really need a unit test that mimics the behavior utilized > by the XLA compiler, for

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

tejohnson added a comment. In 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

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

mehdi_amini added a comment. 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] D77632: [TLI] Per-function fveclib for math library used for vectorization

tejohnson added a comment. In D77632#1974363 , @wenlei wrote: > In D77632#1974015 , @nikic wrote: > > > This change causes a ~0.5% compile-time regressions: > >

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei added a comment. In D77632#1974015 , @nikic wrote: > This change causes a ~0.5% compile-time regressions: >

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

nikic added a comment. This change causes a ~0.5% compile-time regressions: http://llvm-compile-time-tracker.com/compare.php?from=5b18b6e9a84d985c0a907009fb71de7c1943bc88=60c642e74be6af86906d9f3d982728be7bd4329f=instructions This is quite a lot as these things go, so it would be great if you

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

This revision was automatically updated to reflect the committed changes. Closed by commit rG60c642e74be6: [TLI] Per-function fveclib for math library used for vectorization (authored by wenlei). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei updated this revision to Diff 256465. wenlei marked an inline comment as done. wenlei added a comment. rebase, update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/ https://reviews.llvm.org/D77632 Files:

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm. I think one check is missing in the test, see comment below. Comment at: llvm/test/Transforms/Inline/veclib-compat.ll:28 + %rslt = call i32 @callee_massv(i8

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

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: > wenlei wrote: > > tejohnson wrote: > > > Suggest

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei updated this revision to Diff 256437. wenlei marked an inline comment as done. wenlei added a comment. Herald added subscribers: haicheng, eraman. address feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

tejohnson added inline comments. Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:272 +else + VectLibrary = Impl.ClVectorLibrary; + wenlei wrote: > tejohnson wrote: > > Suggest moving the implementation of this constructor to the .cpp file, in

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

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

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

tejohnson added a comment. Great, thanks! A few minor comments below. Looks like it needs a clang-format too. Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:272 +else + VectLibrary = Impl.ClVectorLibrary; + Suggest moving the

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei updated this revision to Diff 256350. wenlei added a comment. address feedback, allow functions within a module to have different vectlib setting. add test case for inline compatibility check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei added a comment. Thanks for taking a look and the suggestions, @tejohnson. Yeah, you're right about the potential conflict of attributes. Initially I thought even though we now allow this to be per-function, but since it comes from per-module switch, module level consistency can still

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

tejohnson added a comment. Needs testing of the inline handling, and of LTO linking IR with different attributes (which is going to hit your assert, see below). Comment at: clang/lib/CodeGen/CGCall.cpp:1987 + // Attach "vect-lib" attribute to function based on '-fveclib'

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei updated this revision to Diff 255703. wenlei added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/ https://reviews.llvm.org/D77632 Files: clang/lib/CodeGen/CGCall.cpp clang/test/CodeGen/libcalls-veclib.c

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei updated this revision to Diff 255679. wenlei added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/ https://reviews.llvm.org/D77632 Files: clang/lib/CodeGen/CGCall.cpp clang/test/CodeGen/libcalls-veclib.c

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

wenlei created this revision. Herald added subscribers: cfe-commits, dexonsmith, hiraditya. Herald added a project: clang. wenlei edited the summary of this revision. wenlei added reviewers: tejohnson, hoyFB, spatel, gchatelet. Encode `-fveclib` setting as per-function attribute so it can be