Enable vectorization of libpgmath intrinsic calls #29

vjayathirtha-nv · 2018-07-20T00:00:08Z

This PR adds PGMATH as one of the values for -fveclib flag.
Also makes -fveclib=PGMATH the default option when
a. User has not explicitly provided the -fveclib option.
b. The optimization level supports vectorization.

vjayathirtha-nv · 2018-07-20T00:00:46Z

Associated with this PR

sscalpone · 2018-07-20T00:02:38Z

lib/Driver/ToolChains/Clang.cpp

+    Args.AddLastArg(CmdArgs, options::OPT_fveclib);
+  else if (Args.hasFlag(options::OPT_fvectorize, VectorizeAliasOption,
+                        options::OPT_fno_vectorize, EnableVec))
+    CmdArgs.push_back("-fveclib=PGMATH");


What happens if someone is using a version of LLVM without support for -fveclib=PGMATH?

LLVM generates an error as it would with any option which it does not recognize.

While flang has its own fork of llvm, flang still works with upstream llvm as well. The changes for DWARF, for example, are enabled with a compile-time #ifdef in flang-compiler/flang. Likely we need a similar mechanism here until we know that all of the upstream guides to building flang (not just blogs, but spack etc) are updated to refer to flang-compiler/flang.

There is already a CMAKE variable that flang builders can use to include Flang-specific LLVM extensions. (FLANG_LLVM_EXTENSIONS)

I don't agree that -fvectorize should imply extra hidden arguments. The user should be in control of the kind of vectorization and the library calls to be used and these should remain independent.

I thought we wanted -fveclib=PGMATH to be the default. I can undo this.

We do want PGMATH to be the default but the flang ecosystem is not ready for that b/c we still want to build with llvm from llvm.org or pre-installed llvm in a distro; we need to move these changes upstream. Unlike SVML, libpgmath is always available with flang so making it the default will not have any negative user impact.

Please add FLANG_LLVM_EXTENSIONS to flang-driver in a subsequent commit; soon please.

schweitzpgi · 2018-07-20T16:58:22Z

lib/Driver/ToolChains/Clang.cpp

@@ -3286,8 +3286,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
  else
    CmdArgs.push_back(Args.MakeArgString(getToolChain().getThreadModel()));

-  Args.AddLastArg(CmdArgs, options::OPT_fveclib);
-


Why is this moved 1000 lines down?

To reuse the EnableVec and VectorizeAliasOption variables in line 4356.

Why not just put the -fveclib change here. Why do we care about the vectorizer command line being present? It's default on all the time, right?

+#ifdef FLANG_LLVM_EXTENSIONS

if(Args.getLastArg(options::OPT_fveclib))

Args.AddLastArg(CmdArgs, options::OPT_fveclib);

else

CmdArgs.push_back("-fveclib=PGMATH");
+#else

Args.AddLastArg(CmdArgs, options::OPT_fveclib);
+#endif

It doesn't make sense to add the -fveclib=PGMATH flag when there is no vectorization taking place. It is default on only when vectorization is on.

vjayathirtha-nv · 2018-07-20T17:46:26Z

I have removed -fveclib=PGMATH as the default flag.

schweitzpgi · 2018-07-23T20:50:37Z

+1

The assertion prevents it from applying fixes when used along with compilation databases with relative paths. Added a test that demonstrates the assertion failure. An example of the assertion: input.cpp:11:14: error: expected ';' after top level declarator typedef int T ^ ; input.cpp:11:14: note: FIX-IT applied suggested code changes clang-check: clang/tools/clang-check/ClangCheck.cpp:94: virtual std::string (anonymous namespace)::FixItOptions::RewriteFilename(const std::string &, int &): Assertion `llvm::sys::path::is_absolute(filename) && "clang-fixit expects absolute paths only."' failed. #0 llvm::sys::PrintStackTrace(llvm::raw_ostream&) llvm/lib/Support/Unix/Signals.inc:494:13 flang-compiler#1 llvm::sys::RunSignalHandlers() llvm/lib/Support/Signals.cpp:69:18 flang-compiler#2 SignalHandler(int) llvm/lib/Support/Unix/Signals.inc:357:1 flang-compiler#3 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x110c0) flang-compiler#4 raise (/lib/x86_64-linux-gnu/libc.so.6+0x32fcf) flang-compiler#5 abort (/lib/x86_64-linux-gnu/libc.so.6+0x343fa) flang-compiler#6 (/lib/x86_64-linux-gnu/libc.so.6+0x2be37) flang-compiler#7 (/lib/x86_64-linux-gnu/libc.so.6+0x2bee2) flang-compiler#8 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) flang-compiler#9 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct_aux<char*>(char*, char*, std::__false_type) flang-compiler#10 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*) flang-compiler#11 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) flang-compiler#12 (anonymous namespace)::FixItOptions::RewriteFilename(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int&) clang/tools/clang-check/ClangCheck.cpp:101:0 flang-compiler#13 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_data() const flang-compiler#14 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_is_local() const flang-compiler#15 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose() flang-compiler#16 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() flang-compiler#17 clang::FixItRewriter::WriteFixedFiles(std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >*) clang/lib/Frontend/Rewrite/FixItRewriter.cpp:98:0 flang-compiler#18 std::__shared_ptr<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2>::get() const flang-compiler#19 std::__shared_ptr_access<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2, false, false>::_M_get() const flang-compiler#20 std::__shared_ptr_access<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2, false, false>::operator->() const flang-compiler#21 clang::CompilerInstance::getFrontendOpts() clang/include/clang/Frontend/CompilerInstance.h:290:0 flang-compiler#22 clang::FrontendAction::EndSourceFile() clang/lib/Frontend/FrontendAction.cpp:966:0 flang-compiler#23 __gnu_cxx::__normal_iterator<clang::FrontendInputFile*, std::vector<clang::FrontendInputFile, std::allocator<clang::FrontendInputFile> > >::operator++() flang-compiler#24 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) clang/lib/Frontend/CompilerInstance.cpp:943:0 flang-compiler#25 clang::tooling::FrontendActionFactory::runInvocation(std::shared_ptr<clang::CompilerInvocation>, clang::FileManager*, std::shared_ptr<clang::PCHContainerOperations>, clang::DiagnosticConsumer*) clang/lib/Tooling/Tooling.cpp:369:33 flang-compiler#26 clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, std::shared_ptr<clang::CompilerInvocation>, std::shared_ptr<clang::PCHContainerOperations>) clang/lib/Tooling/Tooling.cpp:344:18 flang-compiler#27 clang::tooling::ToolInvocation::run() clang/lib/Tooling/Tooling.cpp:329:10 flang-compiler#28 clang::tooling::ClangTool::run(clang::tooling::ToolAction*) clang/lib/Tooling/Tooling.cpp:518:11 flang-compiler#29 main clang/tools/clang-check/ClangCheck.cpp:187:15 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@357915 91177308-0d34-0410-b5e6-96231b3b80d8

…heck. The assertion prevents it from applying fixes when used along with compilation databases with relative paths. Added a test that demonstrates the assertion failure. An example of the assertion: input.cpp:11:14: error: expected ';' after top level declarator typedef int T ^ ; input.cpp:11:14: note: FIX-IT applied suggested code changes clang-check: clang/tools/clang-check/ClangCheck.cpp:94: virtual std::string (anonymous namespace)::FixItOptions::RewriteFilename(const std::string &, int &): Assertion `llvm::sys::path::is_absolute(filename) && "clang-fixit expects absolute paths only."' failed. #0 llvm::sys::PrintStackTrace(llvm::raw_ostream&) llvm/lib/Support/Unix/Signals.inc:494:13 flang-compiler#1 llvm::sys::RunSignalHandlers() llvm/lib/Support/Signals.cpp:69:18 flang-compiler#2 SignalHandler(int) llvm/lib/Support/Unix/Signals.inc:357:1 flang-compiler#3 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x110c0) flang-compiler#4 raise (/lib/x86_64-linux-gnu/libc.so.6+0x32fcf) flang-compiler#5 abort (/lib/x86_64-linux-gnu/libc.so.6+0x343fa) flang-compiler#6 (/lib/x86_64-linux-gnu/libc.so.6+0x2be37) flang-compiler#7 (/lib/x86_64-linux-gnu/libc.so.6+0x2bee2) flang-compiler#8 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) flang-compiler#9 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct_aux<char*>(char*, char*, std::__false_type) flang-compiler#10 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*) flang-compiler#11 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) flang-compiler#12 (anonymous namespace)::FixItOptions::RewriteFilename(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int&) clang/tools/clang-check/ClangCheck.cpp:101:0 flang-compiler#13 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_data() const flang-compiler#14 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_is_local() const flang-compiler#15 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose() flang-compiler#16 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() flang-compiler#17 clang::FixItRewriter::WriteFixedFiles(std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >*) clang/lib/Frontend/Rewrite/FixItRewriter.cpp:98:0 flang-compiler#18 std::__shared_ptr<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2>::get() const flang-compiler#19 std::__shared_ptr_access<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2, false, false>::_M_get() const flang-compiler#20 std::__shared_ptr_access<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2, false, false>::operator->() const flang-compiler#21 clang::CompilerInstance::getFrontendOpts() clang/include/clang/Frontend/CompilerInstance.h:290:0 flang-compiler#22 clang::FrontendAction::EndSourceFile() clang/lib/Frontend/FrontendAction.cpp:966:0 flang-compiler#23 __gnu_cxx::__normal_iterator<clang::FrontendInputFile*, std::vector<clang::FrontendInputFile, std::allocator<clang::FrontendInputFile> > >::operator++() flang-compiler#24 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) clang/lib/Frontend/CompilerInstance.cpp:943:0 flang-compiler#25 clang::tooling::FrontendActionFactory::runInvocation(std::shared_ptr<clang::CompilerInvocation>, clang::FileManager*, std::shared_ptr<clang::PCHContainerOperations>, clang::DiagnosticConsumer*) clang/lib/Tooling/Tooling.cpp:369:33 flang-compiler#26 clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, std::shared_ptr<clang::CompilerInvocation>, std::shared_ptr<clang::PCHContainerOperations>) clang/lib/Tooling/Tooling.cpp:344:18 flang-compiler#27 clang::tooling::ToolInvocation::run() clang/lib/Tooling/Tooling.cpp:329:10 flang-compiler#28 clang::tooling::ClangTool::run(clang::tooling::ToolAction*) clang/lib/Tooling/Tooling.cpp:518:11 flang-compiler#29 main clang/tools/clang-check/ClangCheck.cpp:187:15 ........ Breaks windows buildbots git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@357918 91177308-0d34-0410-b5e6-96231b3b80d8

Re-commit r357915 with a fix for windows. The assertion prevents it from applying fixes when used along with compilation databases with relative paths. Added a test that demonstrates the assertion failure. An example of the assertion: input.cpp:11:14: error: expected ';' after top level declarator typedef int T ^ ; input.cpp:11:14: note: FIX-IT applied suggested code changes clang-check: clang/tools/clang-check/ClangCheck.cpp:94: virtual std::string (anonymous namespace)::FixItOptions::RewriteFilename(const std::string &, int &): Assertion `llvm::sys::path::is_absolute(filename) && "clang-fixit expects absolute paths only."' failed. #0 llvm::sys::PrintStackTrace(llvm::raw_ostream&) llvm/lib/Support/Unix/Signals.inc:494:13 flang-compiler#1 llvm::sys::RunSignalHandlers() llvm/lib/Support/Signals.cpp:69:18 flang-compiler#2 SignalHandler(int) llvm/lib/Support/Unix/Signals.inc:357:1 flang-compiler#3 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x110c0) flang-compiler#4 raise (/lib/x86_64-linux-gnu/libc.so.6+0x32fcf) flang-compiler#5 abort (/lib/x86_64-linux-gnu/libc.so.6+0x343fa) flang-compiler#6 (/lib/x86_64-linux-gnu/libc.so.6+0x2be37) flang-compiler#7 (/lib/x86_64-linux-gnu/libc.so.6+0x2bee2) flang-compiler#8 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) flang-compiler#9 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct_aux<char*>(char*, char*, std::__false_type) flang-compiler#10 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*) flang-compiler#11 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) flang-compiler#12 (anonymous namespace)::FixItOptions::RewriteFilename(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int&) clang/tools/clang-check/ClangCheck.cpp:101:0 flang-compiler#13 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_data() const flang-compiler#14 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_is_local() const flang-compiler#15 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose() flang-compiler#16 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() flang-compiler#17 clang::FixItRewriter::WriteFixedFiles(std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >*) clang/lib/Frontend/Rewrite/FixItRewriter.cpp:98:0 flang-compiler#18 std::__shared_ptr<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2>::get() const flang-compiler#19 std::__shared_ptr_access<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2, false, false>::_M_get() const flang-compiler#20 std::__shared_ptr_access<clang::CompilerInvocation, (__gnu_cxx::_Lock_policy)2, false, false>::operator->() const flang-compiler#21 clang::CompilerInstance::getFrontendOpts() clang/include/clang/Frontend/CompilerInstance.h:290:0 flang-compiler#22 clang::FrontendAction::EndSourceFile() clang/lib/Frontend/FrontendAction.cpp:966:0 flang-compiler#23 __gnu_cxx::__normal_iterator<clang::FrontendInputFile*, std::vector<clang::FrontendInputFile, std::allocator<clang::FrontendInputFile> > >::operator++() flang-compiler#24 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) clang/lib/Frontend/CompilerInstance.cpp:943:0 flang-compiler#25 clang::tooling::FrontendActionFactory::runInvocation(std::shared_ptr<clang::CompilerInvocation>, clang::FileManager*, std::shared_ptr<clang::PCHContainerOperations>, clang::DiagnosticConsumer*) clang/lib/Tooling/Tooling.cpp:369:33 flang-compiler#26 clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, std::shared_ptr<clang::CompilerInvocation>, std::shared_ptr<clang::PCHContainerOperations>) clang/lib/Tooling/Tooling.cpp:344:18 flang-compiler#27 clang::tooling::ToolInvocation::run() clang/lib/Tooling/Tooling.cpp:329:10 flang-compiler#28 clang::tooling::ClangTool::run(clang::tooling::ToolAction*) clang/lib/Tooling/Tooling.cpp:518:11 flang-compiler#29 main clang/tools/clang-check/ClangCheck.cpp:187:15 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@357921 91177308-0d34-0410-b5e6-96231b3b80d8

vjayathirtha-nv mentioned this pull request Jul 20, 2018

Enable vectorization of libpgmath intrinsic calls flang-compiler/llvm#43

Merged

sscalpone reviewed Jul 20, 2018

View reviewed changes

sscalpone requested a review from schweitzpgi July 20, 2018 00:20

schweitzpgi reviewed Jul 20, 2018

View reviewed changes

vjayathirtha-nv force-pushed the release_60 branch from 76295c8 to 8ee2e56 Compare July 20, 2018 17:45

vjayathirtha-nv force-pushed the release_60 branch 3 times, most recently from 440014f to 48cf4f0 Compare July 20, 2018 21:47

Enable vectorization of libpgmath intrinsic calls

d741712

vjayathirtha-nv force-pushed the release_60 branch from 48cf4f0 to d741712 Compare July 23, 2018 20:18

sscalpone merged commit f0856f2 into flang-compiler:release_60 Aug 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable vectorization of libpgmath intrinsic calls #29

Enable vectorization of libpgmath intrinsic calls #29

vjayathirtha-nv commented Jul 20, 2018 •

edited

Loading

vjayathirtha-nv commented Jul 20, 2018

sscalpone Jul 20, 2018

vjayathirtha-nv Jul 20, 2018

sscalpone Jul 20, 2018

schweitzpgi Jul 20, 2018

schweitzpgi Jul 20, 2018

vjayathirtha-nv Jul 20, 2018

sscalpone Jul 20, 2018

sscalpone Jul 20, 2018

schweitzpgi Jul 20, 2018

vjayathirtha-nv Jul 20, 2018

schweitzpgi Jul 20, 2018

vjayathirtha-nv Jul 20, 2018 •

edited

Loading

vjayathirtha-nv commented Jul 20, 2018

schweitzpgi commented Jul 23, 2018

Enable vectorization of libpgmath intrinsic calls #29

Enable vectorization of libpgmath intrinsic calls #29

Conversation

vjayathirtha-nv commented Jul 20, 2018 • edited Loading

vjayathirtha-nv commented Jul 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjayathirtha-nv Jul 20, 2018 • edited Loading

Choose a reason for hiding this comment

vjayathirtha-nv commented Jul 20, 2018

schweitzpgi commented Jul 23, 2018

vjayathirtha-nv commented Jul 20, 2018 •

edited

Loading

vjayathirtha-nv Jul 20, 2018 •

edited

Loading