[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 15 01:54:20 PST 2018


hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

The clang-tidy code looks good. If no one has further comments, feel free to commit it.



================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:46
+
+  static const llvm::StringMap<StringRef> Mapping{
+    // [simd.alg]
----------------
MaskRay wrote:
> hokein wrote:
> > consider using `llvm::StringSwitch`?
> The list is currently a scaffold and more operations will be added. It may be more efficient to use a lookup table instead of cascading `.Case("add", ...)`?
I see.  In your case, the normal switch-case statement seems enough? But up to you.


================
Comment at: docs/clang-tidy/checks/readability-simd-intrinsics.rst:8
+
+If the option ``UseStdExperimental`` is set to non-zero, for
+
----------------
nit: I think it is stale?


================
Comment at: test/clang-tidy/readability-simd-intrinsics-ppc.cpp:3
+// RUN:  -config='{CheckOptions: [ \
+// RUN:    {key: readability-simd-intrinsics.Suggest, value: 1} \
+// RUN:  ]}' -- -target ppc64le -maltivec -std=c++11
----------------
consider adding test when Suggest is 0?


================
Comment at: test/clang-tidy/readability-simd-intrinsics-x86.cpp:25
+  _mm_add_fake(0, 1);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:
+}
----------------
nit: this is not needed. If clang-tidy gives the warning for this case, the lit test will fail.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42983





More information about the cfe-commits mailing list