Merge pull request #175 from sx-aurora-dev/feature/merge-upstream-20220325

Feature/merge upstream 20220325

Author: kaz7

bolt/lib/Passes/LoopInversionPass.cpp

@@ -54,11 +54,16 @@ bool LoopInversionPass::runOnFunction(BinaryFunction &BF) {
         }
       }
 
-      assert(SecondSucc != nullptr && "Unable to find second BB successor");
-      const uint64_t BBCount = SuccBB->getBranchInfo(*BB).Count;
-      const uint64_t OtherCount = SuccBB->getBranchInfo(*SecondSucc).Count;
-      if ((BBCount < OtherCount) && (BBIndex > SuccBBIndex))
+      assert(SecondSucc != nullptr && "Unable to find a second BB successor");
+      const uint64_t LoopCount = SuccBB->getBranchInfo(*BB).Count;
+      const uint64_t ExitCount = SuccBB->getBranchInfo(*SecondSucc).Count;
+
+      if (LoopCount < ExitCount) {
+        if (BBIndex > SuccBBIndex)
+          continue;
+      } else if (BBIndex < SuccBBIndex) {
         continue;
+      }
 
       IsChanged = true;
       BB->setLayoutIndex(SuccBBIndex);

bolt/test/X86/loop-inversion-pass.s

@@ -4,24 +4,33 @@
 # RUN:   %s -o %t.o
 # RUN: link_fdata %s %t.o %t.fdata
 # RUN: link_fdata %s %t.o %t.fdata2 "FDATA2"
+# RUN: link_fdata %s %t.o %t.fdata3 "FDATA3"
 # RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
 # RUN: llvm-bolt %t.exe -data %t.fdata -reorder-blocks=cache+ -print-finalized \
 # RUN:    -loop-inversion-opt -o %t.out | FileCheck %s
 # RUN: llvm-bolt %t.exe -data %t.fdata2 -reorder-blocks=cache+ -print-finalized \
 # RUN:    -loop-inversion-opt -o %t.out2 | FileCheck --check-prefix="CHECK2" %s
+# RUN: llvm-bolt %t.exe -data %t.fdata3 -reorder-blocks=none -print-finalized \
+# RUN:    -loop-inversion-opt -o %t.out3 | FileCheck --check-prefix="CHECK3" %s
 
-# The case where loop is used:
+# The case where the loop is used:
 # FDATA: 1 main 2 1 main #.J1# 0 420
 # FDATA: 1 main b 1 main #.Jloop# 0 420
 # FDATA: 1 main b 1 main d 0 1
 # CHECK: BB Layout   : .LBB00, .Ltmp0, .Ltmp1, .LFT0
 
-# The case where loop is unused:
+# The case where the loop is unused:
 # FDATA2: 1 main 2 1 main #.J1# 0 420
 # FDATA2: 1 main b 1 main #.Jloop# 0 1
 # FDATA2: 1 main b 1 main d 0 420
 # CHECK2: BB Layout   : .LBB00, .Ltmp1, .LFT0, .Ltmp0
 
+# The case where the loop does not require rotation:
+# FDATA3: 1 main 2 1 main #.J1# 0 420
+# FDATA3: 1 main b 1 main #.Jloop# 0 420
+# FDATA3: 1 main b 1 main d 0 1
+# CHECK3: BB Layout   : .LBB00, .Ltmp0, .Ltmp1, .LFT0
+
     .text
     .globl main
     .type main, %function

clang-tools-extra/README.txt

@@ -11,8 +11,8 @@ This repository is only intended to be checked out inside of a full LLVM+Clang
 tree, and in the 'tools/extra' subdirectory of the Clang checkout.
 
 All discussion regarding Clang, Clang-based tools, and code in this repository
-should be held using the standard Clang mailing lists:
-  http://lists.llvm.org/mailman/listinfo/cfe-dev
+should be held using the standard Clang forum:
+  https://discourse.llvm.org/c/clang
 
 Code review for this tree should take place on the standard Clang patch and
 commit lists:

clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp

@@ -23,7 +23,7 @@ namespace clang {
 namespace tidy {
 namespace abseil {
 
-RewriteRule CleanupCtadCheckImpl() {
+RewriteRuleWith<std::string> CleanupCtadCheckImpl() {
   auto warning_message = cat("prefer absl::Cleanup's class template argument "
                              "deduction pattern in C++17 and higher");
 

clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp

@@ -30,7 +30,7 @@ using ::clang::transformer::cat;
 using ::clang::transformer::changeTo;
 using ::clang::transformer::makeRule;
 using ::clang::transformer::node;
-using ::clang::transformer::RewriteRule;
+using ::clang::transformer::RewriteRuleWith;
 
 AST_MATCHER(Type, isCharType) { return Node.isCharType(); }
 
@@ -39,7 +39,7 @@ static const char DefaultStringLikeClasses[] = "::std::basic_string;"
                                                "::absl::string_view";
 static const char DefaultAbseilStringsMatchHeader[] = "absl/strings/match.h";
 
-static transformer::RewriteRule
+static transformer::RewriteRuleWith<std::string>
 makeRewriteRule(const std::vector<std::string> &StringLikeClassNames,
                 StringRef AbseilStringsMatchHeader) {
   auto StringLikeClass = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 4>(
@@ -62,7 +62,7 @@ makeRewriteRule(const std::vector<std::string> &StringLikeClassNames,
             hasArgument(1, cxxDefaultArgExpr())),
       onImplicitObjectArgument(expr().bind("string_being_searched")));
 
-  RewriteRule Rule = applyFirst(
+  RewriteRuleWith<std::string> Rule = applyFirst(
       {makeRule(
            binaryOperator(hasOperatorName("=="),
                           hasOperands(ignoringParenImpCasts(StringNpos),

clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp

@@ -37,7 +37,7 @@ AST_MATCHER(clang::VarDecl, isDirectInitialization) {
 
 } // namespace
 
-RewriteRule StringviewNullptrCheckImpl() {
+RewriteRuleWith<std::string> StringviewNullptrCheckImpl() {
   auto construction_warning =
       cat("constructing basic_string_view from null is undefined; replace with "
           "the default constructor");

clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp

@@ -569,6 +569,7 @@ matchRelationalIntegerConstantExpr(StringRef Id) {
   std::string SwapId = (Id + "-swap").str();
   std::string NegateId = (Id + "-negate").str();
   std::string OverloadId = (Id + "-overload").str();
+  std::string ConstId = (Id + "-const").str();
 
   const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
       isComparisonOperator(), expr().bind(Id),
@@ -600,7 +601,9 @@ matchRelationalIntegerConstantExpr(StringRef Id) {
       cxxOperatorCallExpr(
           hasAnyOverloadedOperatorName("==", "!=", "<", "<=", ">", ">="),
           // Filter noisy false positives.
-          unless(isMacro()), unless(isInTemplateInstantiation()))
+          unless(isMacro()), unless(isInTemplateInstantiation()),
+          anyOf(hasLHS(ignoringParenImpCasts(integerLiteral().bind(ConstId))),
+                hasRHS(ignoringParenImpCasts(integerLiteral().bind(ConstId)))))
           .bind(OverloadId);
 
   return anyOf(RelationalExpr, CastExpr, NegateRelationalExpr,
@@ -674,16 +677,38 @@ static bool retrieveRelationalIntegerConstantExpr(
     if (canOverloadedOperatorArgsBeModified(OverloadedOperatorExpr, false))
       return false;
 
+    bool IntegerConstantIsFirstArg = false;
+
     if (const auto *Arg = OverloadedOperatorExpr->getArg(1)) {
       if (!Arg->isValueDependent() &&
-          !Arg->isIntegerConstantExpr(*Result.Context))
-        return false;
-    }
-    Symbol = OverloadedOperatorExpr->getArg(0);
+          !Arg->isIntegerConstantExpr(*Result.Context)) {
+        IntegerConstantIsFirstArg = true;
+        if (const auto *Arg = OverloadedOperatorExpr->getArg(0)) {
+          if (!Arg->isValueDependent() &&
+              !Arg->isIntegerConstantExpr(*Result.Context))
+            return false;
+        } else
+          return false;
+      }
+    } else
+      return false;
+
+    Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0);
     OperandExpr = OverloadedOperatorExpr;
     Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
 
-    return BinaryOperator::isComparisonOp(Opcode);
+    if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
+      return false;
+
+    if (!BinaryOperator::isComparisonOp(Opcode))
+      return false;
+
+    // The call site of this function expects the constant on the RHS,
+    // so change the opcode accordingly.
+    if (IntegerConstantIsFirstArg)
+      Opcode = BinaryOperator::reverseComparisonOp(Opcode);
+
+    return true;
   } else {
     return false;
   }

clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp

@@ -66,6 +66,13 @@ class FindUsageOfThis : public RecursiveASTVisitor<FindUsageOfThis> {
     return Parents.begin()->get<T>();
   }
 
+  const Expr *getParentExprIgnoreParens(const Expr *E) {
+    const Expr *Parent = getParent<Expr>(E);
+    while (isa_and_nonnull<ParenExpr>(Parent))
+      Parent = getParent<Expr>(Parent);
+    return Parent;
+  }
+
   bool VisitUnresolvedMemberExpr(const UnresolvedMemberExpr *) {
     // An UnresolvedMemberExpr might resolve to a non-const non-static
     // member function.
@@ -140,7 +147,7 @@ class FindUsageOfThis : public RecursiveASTVisitor<FindUsageOfThis> {
       return true;
     }
 
-    const auto *Parent = getParent<Expr>(Member);
+    const auto *Parent = getParentExprIgnoreParens(Member);
 
     if (const auto *Cast = dyn_cast_or_null<ImplicitCastExpr>(Parent)) {
       // A read access to a member is safe when the member either
@@ -167,12 +174,12 @@ class FindUsageOfThis : public RecursiveASTVisitor<FindUsageOfThis> {
   bool VisitCXXThisExpr(const CXXThisExpr *E) {
     Usage = Const;
 
-    const auto *Parent = getParent<Expr>(E);
+    const auto *Parent = getParentExprIgnoreParens(E);
 
     // Look through deref of this.
     if (const auto *UnOp = dyn_cast_or_null<UnaryOperator>(Parent)) {
       if (UnOp->getOpcode() == UO_Deref) {
-        Parent = getParent<Expr>(UnOp);
+        Parent = getParentExprIgnoreParens(UnOp);
       }
     }
 

clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp

@@ -13,16 +13,16 @@
 namespace clang {
 namespace tidy {
 namespace utils {
-using transformer::RewriteRule;
+using transformer::RewriteRuleWith;
 
 #ifndef NDEBUG
-static bool hasExplanation(const RewriteRule::Case &C) {
-  return C.Explanation != nullptr;
+static bool hasGenerator(const transformer::Generator<std::string> &G) {
+  return G != nullptr;
 }
 #endif
 
-static void verifyRule(const RewriteRule &Rule) {
-  assert(llvm::all_of(Rule.Cases, hasExplanation) &&
+static void verifyRule(const RewriteRuleWith<std::string> &Rule) {
+  assert(llvm::all_of(Rule.Metadata, hasGenerator) &&
          "clang-tidy checks must have an explanation by default;"
          " explicitly provide an empty explanation if none is desired");
 }
@@ -39,23 +39,24 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
 // we would be accessing `getLangOpts` and `Options` before the underlying
 // `ClangTidyCheck` instance was properly initialized.
 TransformerClangTidyCheck::TransformerClangTidyCheck(
-    std::function<Optional<RewriteRule>(const LangOptions &,
-                                        const OptionsView &)>
+    std::function<Optional<RewriteRuleWith<std::string>>(const LangOptions &,
+                                                         const OptionsView &)>
         MakeRule,
     StringRef Name, ClangTidyContext *Context)
     : TransformerClangTidyCheck(Name, Context) {
-  if (Optional<RewriteRule> R = MakeRule(getLangOpts(), Options))
+  if (Optional<RewriteRuleWith<std::string>> R =
+          MakeRule(getLangOpts(), Options))
     setRule(std::move(*R));
 }
 
-TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
-                                                     StringRef Name,
-                                                     ClangTidyContext *Context)
+TransformerClangTidyCheck::TransformerClangTidyCheck(
+    RewriteRuleWith<std::string> R, StringRef Name, ClangTidyContext *Context)
     : TransformerClangTidyCheck(Name, Context) {
   setRule(std::move(R));
 }
 
-void TransformerClangTidyCheck::setRule(transformer::RewriteRule R) {
+void TransformerClangTidyCheck::setRule(
+    transformer::RewriteRuleWith<std::string> R) {
   verifyRule(R);
   Rule = std::move(R);
 }
@@ -77,8 +78,9 @@ void TransformerClangTidyCheck::check(
   if (Result.Context->getDiagnostics().hasErrorOccurred())
     return;
 
-  RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, Rule);
-  Expected<SmallVector<transformer::Edit, 1>> Edits = Case.Edits(Result);
+  size_t I = transformer::detail::findSelectedCase(Result, Rule);
+  Expected<SmallVector<transformer::Edit, 1>> Edits =
+      Rule.Cases[I].Edits(Result);
   if (!Edits) {
     llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError())
                  << "\n";
@@ -89,7 +91,7 @@ void TransformerClangTidyCheck::check(
   if (Edits->empty())
     return;
 
-  Expected<std::string> Explanation = Case.Explanation->eval(Result);
+  Expected<std::string> Explanation = Rule.Metadata[I]->eval(Result);
   if (!Explanation) {
     llvm::errs() << "Error in explanation: "
                  << llvm::toString(Explanation.takeError()) << "\n";

clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h

@@ -48,15 +48,16 @@ class TransformerClangTidyCheck : public ClangTidyCheck {
   /// cases where the options disable the check.
   ///
   /// See \c setRule for constraints on the rule.
-  TransformerClangTidyCheck(std::function<Optional<transformer::RewriteRule>(
-                                const LangOptions &, const OptionsView &)>
-                                MakeRule,
-                            StringRef Name, ClangTidyContext *Context);
+  TransformerClangTidyCheck(
+      std::function<Optional<transformer::RewriteRuleWith<std::string>>(
+          const LangOptions &, const OptionsView &)>
+          MakeRule,
+      StringRef Name, ClangTidyContext *Context);
 
   /// Convenience overload of the constructor when the rule doesn't have any
   /// dependencies.
-  TransformerClangTidyCheck(transformer::RewriteRule R, StringRef Name,
-                            ClangTidyContext *Context);
+  TransformerClangTidyCheck(transformer::RewriteRuleWith<std::string> R,
+                            StringRef Name, ClangTidyContext *Context);
 
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override;
@@ -74,10 +75,10 @@ class TransformerClangTidyCheck : public ClangTidyCheck {
   /// is a bug.  If no explanation is desired, indicate that explicitly (for
   /// example, by passing `text("no explanation")` to `makeRule` as the
   /// `Explanation` argument).
-  void setRule(transformer::RewriteRule R);
+  void setRule(transformer::RewriteRuleWith<std::string> R);
 
 private:
-  transformer::RewriteRule Rule;
+  transformer::RewriteRuleWith<std::string> Rule;
   IncludeInserter Inserter;
 };