Skip to content

Commit 258a959

Browse files
MaskRayMaskRay
committed
[clang-tidy] Add readability-simd-intrinsics check.
Summary: 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 http://wg21.link/p0214 for introduction and motivation for the data-parallel standard library. Subscribers: klimek, aemerson, mgorny, xazax.hun, kristof.beyls, hintonda, cfe-commits Differential Revision: https://reviews.llvm.org/D42983 llvm-svn: 325272
1 parent b2c508b commit 258a959

File tree

9 files changed

+283
-0
lines changed

9 files changed

+283
-0
lines changed

‎clang-tools-extra/clang-tidy/readability/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ add_clang_library(clangTidyReadabilityModule
2424
RedundantStringCStrCheck.cpp
2525
RedundantSmartptrGetCheck.cpp
2626
RedundantStringInitCheck.cpp
27+
SIMDIntrinsicsCheck.cpp
2728
SimplifyBooleanExprCheck.cpp
2829
StaticAccessedThroughInstanceCheck.cpp
2930
StaticDefinitionInAnonymousNamespaceCheck.cpp

‎clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "RedundantSmartptrGetCheck.h"
3232
#include "RedundantStringCStrCheck.h"
3333
#include "RedundantStringInitCheck.h"
34+
#include "SIMDIntrinsicsCheck.h"
3435
#include "SimplifyBooleanExprCheck.h"
3536
#include "StaticAccessedThroughInstanceCheck.h"
3637
#include "StaticDefinitionInAnonymousNamespaceCheck.h"
@@ -92,6 +93,8 @@ class ReadabilityModule : public ClangTidyModule {
9293
"readability-redundant-string-cstr");
9394
CheckFactories.registerCheck<RedundantStringInitCheck>(
9495
"readability-redundant-string-init");
96+
CheckFactories.registerCheck<SIMDIntrinsicsCheck>(
97+
"readability-simd-intrinsics");
9598
CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
9699
"readability-simplify-boolean-expr");
97100
CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(

‎clang-tools-extra/clang-tidy/readability/SIMDIntrinsicsCheck.cpp

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
//===--- SIMDIntrinsicsCheck.cpp - clang-tidy------------------------------===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "SIMDIntrinsicsCheck.h"
11+
#include "clang/AST/ASTContext.h"
12+
#include "clang/ASTMatchers/ASTMatchFinder.h"
13+
#include "llvm/ADT/StringMap.h"
14+
#include "llvm/ADT/Triple.h"
15+
#include "llvm/Support/Regex.h"
16+
17+
using namespace clang::ast_matchers;
18+
19+
namespace clang {
20+
namespace tidy {
21+
namespace readability {
22+
23+
namespace {
24+
25+
// If the callee has parameter of VectorType or pointer to VectorType,
26+
// or the return type is VectorType, we consider it a vector function
27+
// and a candidate for checking.
28+
AST_MATCHER(FunctionDecl, isVectorFunction) {
29+
bool IsVector = Node.getReturnType()->isVectorType();
30+
for (const ParmVarDecl *Parm : Node.parameters()) {
31+
QualType Type = Parm->getType();
32+
if (Type->isPointerType())
33+
Type = Type->getPointeeType();
34+
if (Type->isVectorType())
35+
IsVector = true;
36+
}
37+
return IsVector;
38+
}
39+
40+
} // namespace
41+
42+
static StringRef TrySuggestPPC(StringRef Name) {
43+
if (!Name.consume_front("vec_"))
44+
return {};
45+
46+
static const llvm::StringMap<StringRef> Mapping{
47+
// [simd.alg]
48+
{"max", "$std::max"},
49+
{"min", "$std::min"},
50+
51+
// [simd.binary]
52+
{"add", "operator+ on $simd objects"},
53+
{"sub", "operator- on $simd objects"},
54+
{"mul", "operator* on $simd objects"},
55+
};
56+
57+
auto It = Mapping.find(Name);
58+
if (It != Mapping.end())
59+
return It->second;
60+
return {};
61+
}
62+
63+
static StringRef TrySuggestX86(StringRef Name) {
64+
if (!(Name.consume_front("_mm_") || Name.consume_front("_mm256_") ||
65+
Name.consume_front("_mm512_")))
66+
return {};
67+
68+
// [simd.alg]
69+
if (Name.startswith("max_"))
70+
return "$simd::max";
71+
if (Name.startswith("min_"))
72+
return "$simd::min";
73+
74+
// [simd.binary]
75+
if (Name.startswith("add_"))
76+
return "operator+ on $simd objects";
77+
if (Name.startswith("sub_"))
78+
return "operator- on $simd objects";
79+
if (Name.startswith("mul_"))
80+
return "operator* on $simd objects";
81+
82+
return {};
83+
}
84+
85+
SIMDIntrinsicsCheck::SIMDIntrinsicsCheck(StringRef Name,
86+
ClangTidyContext *Context)
87+
: ClangTidyCheck(Name, Context), Suggest(Options.get("Suggest", 0) != 0) {}
88+
89+
void SIMDIntrinsicsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
90+
Options.store(Opts, "Suggest", 0);
91+
}
92+
93+
void SIMDIntrinsicsCheck::registerMatchers(MatchFinder *Finder) {
94+
if (!getLangOpts().CPlusPlus11)
95+
return;
96+
// libcxx implementation backports it to C++11 std::experimental::simd.
97+
Std = getLangOpts().CPlusPlus2a ? "std" : "std::experimental";
98+
99+
Finder->addMatcher(callExpr(callee(functionDecl(allOf(
100+
matchesName("^::(_mm_|_mm256_|_mm512_|vec_)"),
101+
isVectorFunction()))),
102+
unless(isExpansionInSystemHeader()))
103+
.bind("call"),
104+
this);
105+
}
106+
107+
void SIMDIntrinsicsCheck::check(const MatchFinder::MatchResult &Result) {
108+
const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
109+
assert(Call != nullptr);
110+
const FunctionDecl *Callee = Call->getDirectCallee();
111+
if (!Callee)
112+
return;
113+
114+
StringRef Old = Callee->getName();
115+
StringRef New;
116+
llvm::Triple::ArchType Arch =
117+
Result.Context->getTargetInfo().getTriple().getArch();
118+
119+
switch (Arch) {
120+
default:
121+
break;
122+
case llvm::Triple::ppc:
123+
case llvm::Triple::ppc64:
124+
case llvm::Triple::ppc64le:
125+
New = TrySuggestPPC(Old);
126+
break;
127+
case llvm::Triple::x86:
128+
case llvm::Triple::x86_64:
129+
New = TrySuggestX86(Old);
130+
break;
131+
}
132+
133+
if (!New.empty()) {
134+
std::string Message;
135+
// If Suggest is true, give a P0214 alternative, otherwise point it out it
136+
// is non-portable.
137+
if (Suggest) {
138+
Message = (Twine("'") + Old + "' can be replaced by " + New).str();
139+
Message = llvm::Regex("\\$std").sub(Std, Message);
140+
Message = llvm::Regex("\\$simd").sub(Std.str() + "::simd", Message);
141+
} else {
142+
Message = (Twine("'") + Old + "' is a non-portable " +
143+
llvm::Triple::getArchTypeName(Arch) + " intrinsic function")
144+
.str();
145+
}
146+
diag(Call->getExprLoc(), Message);
147+
}
148+
}
149+
150+
} // namespace readability
151+
} // namespace tidy
152+
} // namespace clang

‎clang-tools-extra/clang-tidy/readability/SIMDIntrinsicsCheck.h

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
//===--- SIMDIntrinsicsCheck.h - clang-tidy----------------------*- C++ -*-===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SIMD_INTRINSICS_CHECK_H
11+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SIMD_INTRINSICS_CHECK_H
12+
13+
#include "../ClangTidy.h"
14+
15+
namespace clang {
16+
namespace tidy {
17+
namespace readability {
18+
19+
/// Find SIMD intrinsics calls and suggest std::experimental::simd alternatives.
20+
///
21+
/// For the user-facing documentation see:
22+
/// http://clang.llvm.org/extra/clang-tidy/checks/readability-simd-intrinsics.html
23+
class SIMDIntrinsicsCheck : public ClangTidyCheck {
24+
public:
25+
SIMDIntrinsicsCheck(StringRef Name, ClangTidyContext *Context);
26+
27+
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
28+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
29+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
30+
31+
private:
32+
const bool Suggest;
33+
StringRef Std;
34+
};
35+
36+
} // namespace readability
37+
} // namespace tidy
38+
} // namespace clang
39+
40+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SIMD_INTRINSICS_CHECK_H

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,12 @@ Improvements to clang-tidy
8888
Functions that have trailing returns are disallowed, except for those
8989
using decltype specifiers and lambda with otherwise unutterable
9090
return types.
91+
92+
- New `readability-simd-intrinsics
93+
<http://clang.llvm.org/extra/clang-tidy/checks/readability-simd-intrinsics.html>`_ check
94+
95+
Warns if SIMD intrinsics are used which can be replaced by
96+
``std::experimental::simd`` operations.
9197

9298
- New alias `hicpp-avoid-goto
9399
<http://clang.llvm.org/extra/clang-tidy/checks/hicpp-avoid-goto.html>`_ to

‎clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ Clang-Tidy Checks
217217
readability-redundant-smartptr-get
218218
readability-redundant-string-cstr
219219
readability-redundant-string-init
220+
readability-simd-intrinsics
220221
readability-simplify-boolean-expr
221222
readability-static-accessed-through-instance
222223
readability-static-definition-in-anonymous-namespace

‎clang-tools-extra/docs/clang-tidy/checks/readability-simd-intrinsics.rst

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
.. title:: clang-tidy - readability-simd-intrinsics
2+
3+
readability-simd-intrinsics
4+
===========================
5+
6+
Finds SIMD intrinsics calls and suggests ``std::experimental::simd`` (`P0214`_) alternatives.
7+
8+
If the option ``Suggest`` is set to non-zero, for
9+
10+
.. code-block:: c++
11+
12+
_mm_add_epi32(a, b); // x86
13+
vec_add(a, b); // Power
14+
15+
the check suggests an alternative:
16+
17+
.. code-block::
18+
19+
operator+ on std::experimental::simd objects
20+
21+
Otherwise, it just complains the intrinsics are non-portable (and there are `P0214`_ alternatives).
22+
23+
Many architectures provide SIMD operations (e.g. x86 SSE/AVX, Power AltiVec/VSX,
24+
ARM NEON). It is common that SIMD code implementing the same algorithm, is
25+
written in multiple target-dispatching pieces to optimize for different
26+
architectures or micro-architectures.
27+
28+
The C++ standard proposal `P0214`_ and its extensions cover many common SIMD
29+
operations. By migrating from target-dependent intrinsics to `P0214` operations,
30+
the SIMD code can be simplified and pieces for different targets can be unified.
31+
32+
Refer to `P0214`_ for introduction and motivation for the data-parallel standard
33+
library.
34+
35+
Options
36+
-------
37+
38+
.. option:: Suggest
39+
40+
If this option is set to non-zero (default is `0`), the check will suggest P0214 alternatives, otherwise it only points out the intrinsic function is non-portable.
41+
42+
.. _P0214: http://wg21.link/p0214

‎clang-tools-extra/test/clang-tidy/readability-simd-intrinsics-ppc.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \
2+
// RUN: -config='{CheckOptions: [ \
3+
// RUN: {key: readability-simd-intrinsics.Suggest, value: 1} \
4+
// RUN: ]}' -- -target ppc64le -maltivec -std=c++11
5+
6+
vector int vec_add(vector int, vector int);
7+
8+
void PPC() {
9+
vector int i0, i1;
10+
11+
vec_add(i0, i1);
12+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'vec_add' can be replaced by operator+ on std::experimental::simd objects [readability-simd-intrinsics]
13+
}

‎clang-tools-extra/test/clang-tidy/readability-simd-intrinsics-x86.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \
2+
// RUN: -config='{CheckOptions: [ \
3+
// RUN: {key: readability-simd-intrinsics.Suggest, value: 1} \
4+
// RUN: ]}' -- -target x86_64 -std=c++11
5+
6+
typedef long long __m128i __attribute__((vector_size(16)));
7+
typedef double __m256 __attribute__((vector_size(32)));
8+
9+
__m128i _mm_add_epi32(__m128i, __m128i);
10+
__m256 _mm256_load_pd(double const *);
11+
void _mm256_store_pd(double *, __m256);
12+
13+
int _mm_add_fake(int, int);
14+
15+
void X86() {
16+
__m128i i0, i1;
17+
__m256 d0;
18+
19+
_mm_add_epi32(i0, i1);
20+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can be replaced by operator+ on std::experimental::simd objects [readability-simd-intrinsics]
21+
d0 = _mm256_load_pd(0);
22+
_mm256_store_pd(0, d0);
23+
24+
_mm_add_fake(0, 1);
25+
}

0 commit comments

Comments
 (0)