llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: None (serge-sans-paille) <details> <summary>Changes</summary> Basically turning `x << N | x >> (64 - N)` into `std::rotl(x, N)`. --- Full diff: https://github.com/llvm/llvm-project/pull/186324.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp (+58-3) - (modified) clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst (+2) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp (+56) ``````````diff diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp index e649c1c5aadda..e477bbdf47327 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdBitCheck.cpp @@ -8,6 +8,7 @@ #include "UseStdBitCheck.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/Support/FormatVariadic.h" using namespace clang::ast_matchers; @@ -37,7 +38,10 @@ void UseStdBitCheck::registerMatchers(MatchFinder *Finder) { const auto LogicalAnd = MakeCommutativeBinaryOperatorMatcher("&&"); const auto Sub = MakeBinaryOperatorMatcher("-"); + const auto ShiftLeft = MakeBinaryOperatorMatcher("<<"); + const auto ShiftRight = MakeBinaryOperatorMatcher(">>"); const auto BitwiseAnd = MakeCommutativeBinaryOperatorMatcher("&"); + const auto BitwiseOr = MakeCommutativeBinaryOperatorMatcher("|"); const auto CmpNot = MakeCommutativeBinaryOperatorMatcher("!="); const auto CmpGt = MakeBinaryOperatorMatcher(">"); const auto CmpGte = MakeBinaryOperatorMatcher(">="); @@ -87,6 +91,15 @@ void UseStdBitCheck::registerMatchers(MatchFinder *Finder) { hasArgument(0, expr(hasType(isUnsignedInteger())).bind("v"))))) .bind("popcount_expr"), this); + + // Rotating an integer by a fixed amount + Finder->addMatcher( + BitwiseOr(ShiftLeft(BindDeclRef("v"), + integerLiteral().bind("shift_left_amount")), + ShiftRight(BoundDeclRef("v"), + integerLiteral().bind("shift_right_amount"))) + .bind("rotate_expr"), + this); } void UseStdBitCheck::registerPPCallbacks(const SourceManager &SM, @@ -141,9 +154,51 @@ void UseStdBitCheck::check(const MatchFinder::MatchResult &Result) { << IncludeInserter.createIncludeInsertion( Source.getFileID(MatchedExpr->getBeginLoc()), "<bit>"); } - } else { - llvm_unreachable("unexpected match"); + } else if (const auto *MatchedExpr = + Result.Nodes.getNodeAs<BinaryOperator>("rotate_expr")) { + const auto *MatchedVarDecl = Result.Nodes.getNodeAs<VarDecl>("v"); + const auto ShiftLeftAmount = + Result.Nodes.getNodeAs<IntegerLiteral>("shift_left_amount")->getValue(); + const auto ShiftRightAmount = + Result.Nodes.getNodeAs<IntegerLiteral>("shift_right_amount") + ->getValue(); + const uint64_t MatchedVarSize = + Context.getTypeSize(MatchedVarDecl->getType()); + + // Overflowing shifts + if (ShiftLeftAmount.sge(MatchedVarSize)) + return; + if (ShiftRightAmount.sge(MatchedVarSize)) + return; + // Not a rotation. + if (MatchedVarSize != (ShiftLeftAmount + ShiftRightAmount)) + return; + + const bool NeedsIntCast = + MatchedExpr->getType() != MatchedVarDecl->getType(); + const bool isRotl = ShiftRightAmount.sge(ShiftLeftAmount); + + const StringRef ReplacementFuncName = isRotl ? "rotl" : "rotr"; + const uint64_t ReplacementShiftAmount = + (isRotl ? ShiftLeftAmount : ShiftRightAmount).getZExtValue(); + auto Diag = diag(MatchedExpr->getBeginLoc(), "use 'std::%0' instead") + << ReplacementFuncName; + if (auto R = MatchedExpr->getSourceRange(); + !R.getBegin().isMacroID() && !R.getEnd().isMacroID()) { + Diag << FixItHint::CreateReplacement( + MatchedExpr->getSourceRange(), + llvm::formatv("{3}std::{0}({1}, {2})", ReplacementFuncName, + MatchedVarDecl->getName(), + ReplacementShiftAmount, + NeedsIntCast ? "(int)" : "") + .str()) + << IncludeInserter.createIncludeInsertion( + Source.getFileID(MatchedExpr->getBeginLoc()), "<bit>"); + } + + } else { + llvm_unreachable("unexpected match"); + } } -} } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst index 26ef1b0841654..0428a48c653fe 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-bit.rst @@ -15,4 +15,6 @@ Expression Replacement ``(x != 0) && !(x & (x - 1))`` ``std::has_one_bit(x)`` ``(x > 0) && !(x & (x - 1))`` ``std::has_one_bit(x)`` ``std::bitset<N>(x).count()`` ``std::popcount(x)`` +``x << 3 | x >> 61`` ``std::rotl(x, 3)`` +``x << 61 | x >> 3`` ``std::rotr(x, 3)`` ============================== ======================= diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp index 52c725282f33c..0392fb9b8ec63 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-bit.cpp @@ -195,3 +195,59 @@ unsigned invalid_popcount_bitset(unsigned x, signed y) { }; } + +/* + * rotate patterns + */ +unsigned char rotate_left_pattern(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotl(x, 3); + return x << 3 | x >> 5; +} +unsigned char rotate_left_pattern_perm(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotl(x, 3); + return x >> 5 | x << 3; +} +unsigned char rotate_swap_pattern(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotl' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotl(x, 4); + return x << 4 | x >> 4; +} +unsigned char rotate_right_pattern(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotr(x, 3); + return x << 5 | x >> 3; +} +unsigned char rotate_right_pattern_perm(unsigned char x) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] + // CHECK-FIXES: return (int)std::rotr(x, 3); + return x >> 3 | x << 5; +} + +#define ROTR v >> 3 | v << 5 +unsigned char rotate_macro(unsigned char v) { + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: use 'std::rotr' instead [modernize-use-std-bit] + // No fixes, it comes from macro expansion. + return ROTR; +} + +/* + * Invalid rotate patterns + */ +void invalid_rotate_patterns(unsigned char x, signed char y, unsigned char z) { + int patterns[] = { + // non-matching references + x >> 3 | z << 5, + // bad shift combination + x >> 3 | x << 6, + x >> 4 | x << 3, + // bad operator combination + x << 3 | x << 6, + x + 3 | x << 6, + x >> 3 & x << 5, + x >> 5 ^ x << 3, + // unsupported types + y >> 4 | y << 4, + }; +} `````````` </details> https://github.com/llvm/llvm-project/pull/186324 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
