MaskRay marked 3 inline comments as done.
MaskRay added inline comments.
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:26
+
+ static const llvm::StringMap<std::string> Mapping{
+ // [simd.alg]
----------------
lebedev.ri wrote:
> You probably want to move `Mapping` out of the function.
Does keeping ` static const llvm::StringMap<std::string> Mapping{ ... }`
inside the function avoid a global constructor? The blacklist will surely be
modified to cover more operations after the revision
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:34
+ {"sub", "operator- on std::experimental::simd objects"},
+ {"mul", "operator* on std::experimental::simd objects"},
+ };
----------------
lebedev.ri wrote:
> To point the obvious, this is not exhaustive list, at least `div` is missing.
Yes, the blacklist will be modified to cover more instructions.
Another fun fact is that though `_mm_div_epi*` are listed in Intel's intrinsics
guide, no there are no mapping hardware instructions and the functions only
exist in ICC/ICPC (I guess) not in clang/lib/Headers/*.h
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:55
+ // [simd.binary]
+ if (Name.startswith("add_"))
+ return "operator+ on std::experimental::simd objects";
----------------
lebedev.ri wrote:
> This function was not updated to use the `Mapping` map.
This is because on Power, these functions are overloaded `vec_add` `vec_sub` ...
But on x86, there are suffixes, e.g. `_mm_add_epi32`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42983
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
