MaskRay updated this revision to Diff 133476. MaskRay added a comment. .rst
Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42983 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/SIMDIntrinsicsCheck.cpp clang-tidy/readability/SIMDIntrinsicsCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-simd-intrinsics.rst test/clang-tidy/readability-simd-intrinsics-ppc.cpp test/clang-tidy/readability-simd-intrinsics-x86.cpp
Index: test/clang-tidy/readability-simd-intrinsics-x86.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-simd-intrinsics-x86.cpp @@ -0,0 +1,26 @@ +// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: readability-simd-intrinsics.Enabled, value: 1} \ +// RUN: ]}' -- -target x86_64 -std=c++2a + +typedef long long __m128i __attribute__((vector_size(16))); +typedef double __m256 __attribute__((vector_size(32))); + +__m128i _mm_add_epi32(__m128i, __m128i); +__m256 _mm256_load_pd(double const *); +void _mm256_store_pd(double *, __m256); + +int _mm_add_fake(int, int); + +void X86() { + __m128i i0, i1; + __m256 d0; + + _mm_add_epi32(i0, i1); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can be replaced by operator+ on std::simd objects [readability-simd-intrinsics] + d0 = _mm256_load_pd(0); + _mm256_store_pd(0, d0); + + _mm_add_fake(0, 1); +// CHECK-MESSAGES-NOT: :[[@LINE-1]]: +} Index: test/clang-tidy/readability-simd-intrinsics-ppc.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-simd-intrinsics-ppc.cpp @@ -0,0 +1,13 @@ +// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: readability-simd-intrinsics.Enabled, value: 1} \ +// RUN: ]}' -- -target ppc64le -maltivec -std=c++2a + +vector int vec_add(vector int, vector int); + +void PPC() { + vector int i0, i1; + + vec_add(i0, i1); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'vec_add' can be replaced by operator+ on std::simd objects [readability-simd-intrinsics] +} Index: docs/clang-tidy/checks/readability-simd-intrinsics.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-simd-intrinsics.rst @@ -0,0 +1,41 @@ +.. title:: clang-tidy - readability-simd-intrinsics + +readability-simd-intrinsics +=========================== + +Finds SIMD intrinsics calls and suggests ``std::experimental::simd`` (`P0214`_) alternatives. + +If the option ``Enabled`` is set to non-zero, for + +.. code-block:: c++ + + _mm_add_epi32(a, b); // x86 + vec_add(a, b); // Power + +the check suggests an alternative: + +.. code-block:: + + operator+ on std::experimental::simd objects // -std=c++11 + operator+ on std::simd objects // -std=c++2a + +Many architectures provide SIMD operations (e.g. x86 SSE/AVX, Power AltiVec/VSX, +ARM NEON). It is common that SIMD code implementing the same algorithm, is +written in multiple target-dispatching pieces to optimize for different +architectures or micro-architectures. + +The C++ standard proposal `P0214`_ and its extensions cover many common SIMD +operations. By migrating from target-dependent intrinsics to `P0214` operations, +the SIMD code can be simplified and pieces for different targets can be unified. + +Refer to `P0214`_ for introduction and motivation for the data-parallel standard +library. + +Options +------- + +.. option:: Enabled + + If set to non-zero, the check will be enabled. Default is ``0``. + +.. _P0214: http://wg21.link/p0214 Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -216,6 +216,7 @@ readability-redundant-smartptr-get readability-redundant-string-cstr readability-redundant-string-init + readability-simd-intrinsics readability-simplify-boolean-expr readability-static-accessed-through-instance readability-static-definition-in-anonymous-namespace Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -88,6 +88,12 @@ Functions that have trailing returns are disallowed, except for those using decltype specifiers and lambda with otherwise unutterable return types. + +- New `readability-simd-intrinsics + <http://clang.llvm.org/extra/clang-tidy/checks/readability-simd-intrinsics.html>`_ check + + Warns if SIMD intrinsics are used which can be replaced by + ``std::experimental::simd`` operations. - New alias `hicpp-avoid-goto <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-avoid-goto.html>`_ to Index: clang-tidy/readability/SIMDIntrinsicsCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/SIMDIntrinsicsCheck.h @@ -0,0 +1,40 @@ +//===--- SIMDIntrinsicsCheck.h - clang-tidy----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SIMD_INTRINSICS_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SIMD_INTRINSICS_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Find SIMD intrinsics calls and suggest std::experimental::simd alternatives. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-simd-intrinsics.html +class SIMDIntrinsicsCheck : public ClangTidyCheck { +public: + SIMDIntrinsicsCheck(StringRef Name, ClangTidyContext *Context); + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + + private: + const bool Enabled; + StringRef Std; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SIMD_INTRINSICS_CHECK_H Index: clang-tidy/readability/SIMDIntrinsicsCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/SIMDIntrinsicsCheck.cpp @@ -0,0 +1,149 @@ +//===--- SIMDIntrinsicsCheck.cpp - clang-tidy------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "SIMDIntrinsicsCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/Triple.h" +#include "llvm/Support/Regex.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +// If the callee has parameter of VectorType or pointer to VectorType, +// or the return type is VectorType, we consider it a vector function +// and a candidate for checking. +namespace { + +AST_MATCHER(FunctionDecl, isVectorFunction) { + bool IsVector = Node.getReturnType()->isVectorType(); + for (const ParmVarDecl *Parm : Node.parameters()) { + QualType Type = Parm->getType(); + if (Type->isPointerType()) + Type = Type->getPointeeType(); + if (Type->isVectorType()) + IsVector = true; + } + return IsVector; +} + +} // namespace + +static StringRef TrySuggestPPC(StringRef Name) { + if (!Name.consume_front("vec_")) + return {}; + + static const llvm::StringMap<std::string> Mapping{ + // [simd.alg] + {"max", "$std::max"}, + {"min", "$std::min"}, + + // [simd.binary] + {"add", "operator+ on $simd objects"}, + {"sub", "operator- on $simd objects"}, + {"mul", "operator* on $simd objects"}, + }; + + auto It = Mapping.find(Name); + if (It != Mapping.end()) + return It->second; + return {}; +} + +static StringRef TrySuggestX86(StringRef Name) { + if (!(Name.consume_front("_mm_") || Name.consume_front("_mm256_") || + Name.consume_front("_mm512_"))) + return {}; + + // [simd.alg] + if (Name.startswith("max_")) + return "$simd::max"; + if (Name.startswith("min_")) + return "$simd::min"; + + // [simd.binary] + if (Name.startswith("add_")) + return "operator+ on $simd objects"; + if (Name.startswith("sub_")) + return "operator- on $simd objects"; + if (Name.startswith("mul_")) + return "operator* on $simd objects"; + + return {}; +} + +SIMDIntrinsicsCheck::SIMDIntrinsicsCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Enabled(Options.getLocalOrGlobal("Enabled", 0) != 0) {} + +void SIMDIntrinsicsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "Enabled", Enabled); +} + +void SIMDIntrinsicsCheck::registerMatchers(MatchFinder *Finder) { + if (!Enabled) + return; + if (getLangOpts().CPlusPlus2a) { + Std = "std"; + } else if (getLangOpts().CPlusPlus11) { + // libcxx implementation backports it to C++11 std::experimental::simd. + Std = "std::experimental"; + } else + return; + + Finder->addMatcher(callExpr(callee(functionDecl(allOf( + matchesName("^::(_mm_|_mm256_|_mm512_|vec_)"), + isVectorFunction()))), + unless(isExpansionInSystemHeader())) + .bind("call"), + this); +} + +void SIMDIntrinsicsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); + assert(Call != nullptr); + const FunctionDecl *Callee = Call->getDirectCallee(); + if (!Callee) + return; + + StringRef Old = Callee->getName(); + StringRef New; + + llvm::Triple::ArchType Arch = + Result.Context->getTargetInfo().getTriple().getArch(); + switch (Arch) { + default: + break; + case llvm::Triple::ppc: + case llvm::Triple::ppc64: + case llvm::Triple::ppc64le: + New = TrySuggestPPC(Old); + break; + case llvm::Triple::x86: + case llvm::Triple::x86_64: + New = TrySuggestX86(Old); + break; + } + + if (!New.empty()) { + std::string Message = (Twine("'") + Old + "' can be replaced by " + New).str(); + Message = llvm::Regex("\\$std").sub(Std, Message); + Message = llvm::Regex("\\$simd").sub(Std.str() + "::simd", Message); + diag(Call->getExprLoc(), Message); + } +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -31,6 +31,7 @@ #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "RedundantStringInitCheck.h" +#include "SIMDIntrinsicsCheck.h" #include "SimplifyBooleanExprCheck.h" #include "StaticAccessedThroughInstanceCheck.h" #include "StaticDefinitionInAnonymousNamespaceCheck.h" @@ -92,6 +93,8 @@ "readability-redundant-string-cstr"); CheckFactories.registerCheck<RedundantStringInitCheck>( "readability-redundant-string-init"); + CheckFactories.registerCheck<SIMDIntrinsicsCheck>( + "readability-simd-intrinsics"); CheckFactories.registerCheck<SimplifyBooleanExprCheck>( "readability-simplify-boolean-expr"); CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>( Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -24,6 +24,7 @@ RedundantStringCStrCheck.cpp RedundantSmartptrGetCheck.cpp RedundantStringInitCheck.cpp + SIMDIntrinsicsCheck.cpp SimplifyBooleanExprCheck.cpp StaticAccessedThroughInstanceCheck.cpp StaticDefinitionInAnonymousNamespaceCheck.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits