https://github.com/voyager-jhk created https://github.com/llvm/llvm-project/pull/203757
This patch introduces a new check to detect lambdas that capture local variables by reference and subsequently escape the local scope, leading to use-after-free. It identifies two primary escape mechanisms: concurrency sinks, where lambdas are passed to asynchronous execution APIs (e.g., `std::thread`), and storage sinks, where they are stored in containers (e.g., `std::vector`) that have global or field storage duration. The AST matcher explicitly unwraps the argument conversion spine to accurately target the escaping lambdas. All escape sinks are configurable via `CheckOptions`. >From d84ab5d9baae42fbbf959e5cdbfc2df5b2acda81 Mon Sep 17 00:00:00 2001 From: SiHuaN <[email protected]> Date: Wed, 10 Jun 2026 15:20:16 +0800 Subject: [PATCH] [clang-tidy] Add bugprone-lambda-capture-lifetime check This patch introduces a new check to detect lambdas that capture local variables by reference and subsequently escape the local scope, leading to use-after-free. It identifies two primary escape mechanisms: 1. Concurrency sinks: Passed to asynchronous execution APIs (e.g., std::thread). 2. Storage sinks: Stored in containers (e.g., std::vector) that have global or field storage duration. The AST matcher explicitly unwraps the argument conversion spine to accurately target the escaping lambdas. All escape sinks are configurable via CheckOptions. --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/LambdaCaptureLifetimeCheck.cpp | 153 ++++++++++++++++++ .../bugprone/LambdaCaptureLifetimeCheck.h | 42 +++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../bugprone/lambda-capture-lifetime.rst | 59 +++++++ .../docs/clang-tidy/checks/list.rst | 1 + .../bugprone/lambda-capture-lifetime.cpp | 66 ++++++++ llvm/lib/Target/RISCV/RISCVMoveMerger.cpp | 151 +++++++++++++++-- llvm/test/CodeGen/RISCV/rv32-move-merge.ll | 94 +++++++++++ 10 files changed, 561 insertions(+), 15 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/bugprone/LambdaCaptureLifetimeCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/LambdaCaptureLifetimeCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-capture-lifetime.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-capture-lifetime.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 3aa39d10ceb5d..f10ec5dae5bbb 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -45,6 +45,7 @@ #include "InfiniteLoopCheck.h" #include "IntegerDivisionCheck.h" #include "InvalidEnumDefaultInitializationCheck.h" +#include "LambdaCaptureLifetimeCheck.h" #include "LambdaFunctionNameCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" @@ -184,6 +185,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-incorrect-enable-if"); CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>( "bugprone-incorrect-enable-shared-from-this"); + CheckFactories.registerCheck<LambdaCaptureLifetimeCheck>( + "bugprone-lambda-capture-lifetime"); CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>( "bugprone-unintended-char-ostream-output"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 43e85b1407f21..33a07e3243aa0 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -38,6 +38,7 @@ add_clang_library(clangTidyBugproneModule STATIC IncorrectEnableIfCheck.cpp IncorrectEnableSharedFromThisCheck.cpp InvalidEnumDefaultInitializationCheck.cpp + LambdaCaptureLifetimeCheck.cpp MissingEndComparisonCheck.cpp UnintendedCharOstreamOutputCheck.cpp ReturnConstRefFromParameterCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/LambdaCaptureLifetimeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/LambdaCaptureLifetimeCheck.cpp new file mode 100644 index 0000000000000..bcc03d5120491 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/LambdaCaptureLifetimeCheck.cpp @@ -0,0 +1,153 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "LambdaCaptureLifetimeCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { +namespace { + +const LambdaExpr *getEscapingLambdaFromArgument(const Expr *E) { + if (!E) + return nullptr; + + E = E->IgnoreParenImpCasts(); + + if (const auto *Lambda = dyn_cast<LambdaExpr>(E)) + return Lambda; + + if (const auto *Cleanups = dyn_cast<ExprWithCleanups>(E)) + return getEscapingLambdaFromArgument(Cleanups->getSubExpr()); + + if (const auto *Temporary = dyn_cast<MaterializeTemporaryExpr>(E)) + return getEscapingLambdaFromArgument(Temporary->getSubExpr()); + + if (const auto *Temporary = dyn_cast<CXXBindTemporaryExpr>(E)) + return getEscapingLambdaFromArgument(Temporary->getSubExpr()); + + if (const auto *Cast = dyn_cast<CastExpr>(E)) + return getEscapingLambdaFromArgument(Cast->getSubExpr()); + + if (const auto *Construct = dyn_cast<CXXConstructExpr>(E)) { + if (Construct->getNumArgs() == 1) + return getEscapingLambdaFromArgument(Construct->getArg(0)); + return nullptr; + } + + if (const auto *InitList = dyn_cast<InitListExpr>(E)) { + if (InitList->getNumInits() == 1) + return getEscapingLambdaFromArgument(InitList->getInit(0)); + return nullptr; + } + + return nullptr; +} + +const LambdaExpr *getEscapingLambda(const CXXConstructExpr *Construct) { + for (const Expr *Arg : Construct->arguments()) + if (const LambdaExpr *Lambda = getEscapingLambdaFromArgument(Arg)) + return Lambda; + return nullptr; +} + +const LambdaExpr *getEscapingLambda(const CallExpr *Call) { + for (const Expr *Arg : Call->arguments()) + if (const LambdaExpr *Lambda = getEscapingLambdaFromArgument(Arg)) + return Lambda; + return nullptr; +} + +bool capturesLocalVariableByReference(const LambdaExpr *Lambda) { + for (const LambdaCapture &Capture : Lambda->captures()) { + if (!Capture.capturesVariable() || Capture.getCaptureKind() != LCK_ByRef) + continue; + + if (const auto *Var = dyn_cast<VarDecl>(Capture.getCapturedVar())) { + if (Var->hasLocalStorage()) + return true; + } + } + return false; +} + +} // namespace + +LambdaCaptureLifetimeCheck::LambdaCaptureLifetimeCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AsyncClasses(utils::options::parseStringList( + Options.get("AsyncClasses", "::std::thread;::std::jthread"))), + AsyncFunctions(utils::options::parseStringList( + Options.get("AsyncFunctions", "::std::async"))), + StorageClasses(utils::options::parseStringList( + Options.get("StorageClasses", "::std::vector"))), + StorageFunctions(utils::options::parseStringList(Options.get( + "StorageFunctions", "push_back;emplace_back;insert;assign"))) {} + +void LambdaCaptureLifetimeCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AsyncClasses", + utils::options::serializeStringList(AsyncClasses)); + Options.store(Opts, "AsyncFunctions", + utils::options::serializeStringList(AsyncFunctions)); + Options.store(Opts, "StorageClasses", + utils::options::serializeStringList(StorageClasses)); + Options.store(Opts, "StorageFunctions", + utils::options::serializeStringList(StorageFunctions)); +} + +void LambdaCaptureLifetimeCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(cxxConstructExpr(hasDeclaration(cxxConstructorDecl( + ofClass(hasAnyName(AsyncClasses)))), + hasAnyArgument(expr())) + .bind("escape-point"), + this); + + Finder->addMatcher(callExpr(callee(functionDecl(hasAnyName(AsyncFunctions))), + hasAnyArgument(expr())) + .bind("escape-point"), + this); + + auto LongLivedStorage = anyOf(varDecl(hasGlobalStorage()), fieldDecl()); + + auto IsDirectRef = declRefExpr(to(LongLivedStorage)); + auto IsMemberRef = memberExpr(member(LongLivedStorage)); + auto StorageClass = cxxRecordDecl( + anyOf(hasAnyName(StorageClasses), + classTemplateSpecializationDecl(hasAnyName(StorageClasses)))); + + Finder->addMatcher( + cxxMemberCallExpr( + callee(cxxMethodDecl(hasAnyName(StorageFunctions), + ofClass(StorageClass))), + on(expr(ignoringParenImpCasts(anyOf(IsDirectRef, IsMemberRef)))), + hasAnyArgument(expr())) + .bind("escape-point"), + this); +} + +void LambdaCaptureLifetimeCheck::check(const MatchFinder::MatchResult &Result) { + const LambdaExpr *Lambda = nullptr; + if (const auto *Construct = + Result.Nodes.getNodeAs<CXXConstructExpr>("escape-point")) + Lambda = getEscapingLambda(Construct); + else if (const auto *Call = Result.Nodes.getNodeAs<CallExpr>("escape-point")) + Lambda = getEscapingLambda(Call); + + if (Lambda && capturesLocalVariableByReference(Lambda)) { + diag(Lambda->getBeginLoc(), + "lambda captures local variables by reference, but escapes the local " + "scope, potentially causing a use-after-free"); + } +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/LambdaCaptureLifetimeCheck.h b/clang-tools-extra/clang-tidy/bugprone/LambdaCaptureLifetimeCheck.h new file mode 100644 index 0000000000000..99a455172c2f1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/LambdaCaptureLifetimeCheck.h @@ -0,0 +1,42 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LAMBDACAPTURELIFETIMECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LAMBDACAPTURELIFETIMECHECK_H + +#include "../ClangTidyCheck.h" +#include <vector> + +namespace clang::tidy::bugprone { + +/// Finds lambdas that capture local variables by reference and escape their +/// local scope by being passed to asynchronous sinks or out-of-scope +/// containers. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/lambda-capture-lifetime.html +class LambdaCaptureLifetimeCheck : public ClangTidyCheck { +public: + LambdaCaptureLifetimeCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + +private: + const std::vector<StringRef> AsyncClasses; + const std::vector<StringRef> AsyncFunctions; + const std::vector<StringRef> StorageClasses; + const std::vector<StringRef> StorageFunctions; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LAMBDACAPTURELIFETIMECHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 99fe37f4145dd..079d73f688ad0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -219,6 +219,12 @@ New checks Finds assignments within selection statements. +- New :doc:`bugprone-lambda-capture-lifetime + <clang-tidy/checks/bugprone/lambda-capture-lifetime>` check. + + Finds lambdas that capture local variables by reference and escape their + local scope by being passed to asynchronous sinks or out-of-scope containers. + - New :doc:`bugprone-missing-end-comparison <clang-tidy/checks/bugprone/missing-end-comparison>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-capture-lifetime.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-capture-lifetime.rst new file mode 100644 index 0000000000000..6c3666b41cad1 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-capture-lifetime.rst @@ -0,0 +1,59 @@ +.. title:: clang-tidy - bugprone-lambda-capture-lifetime + +bugprone-lambda-capture-lifetime +================================ + +Finds lambdas that capture local variables by reference and escape their +local scope by being passed to asynchronous sinks or out-of-scope containers, +potentially causing use-after-free bugs. + +Examples: + +.. code-block:: c++ + + #include <thread> + #include <vector> + #include <functional> + + void thread_escape() { + int local_var = 42; + // WARNING: 'local_var' is captured by reference but escapes to a thread + std::thread t([&local_var]() { + local_var++; + }); + t.detach(); + } // 'local_var' is destroyed here, causing a use-after-free in the thread. + + std::vector<std::function<void()>> GlobalActions; + + void container_escape() { + int local_var = 42; + // WARNING: 'local_var' is captured by reference but escapes to a global container + GlobalActions.push_back([&local_var]() { + local_var++; + }); + } // 'local_var' is destroyed here, but the lambda lives on in GlobalActions. + +Options +------- + +.. option:: AsyncClasses + + Semicolon-separated list of names of asynchronous classes whose constructors + are considered escape sinks. Default is ``::std::thread;::std::jthread``. + +.. option:: AsyncFunctions + + Semicolon-separated list of names of asynchronous functions that are + considered escape sinks. Default is ``::std::async``. + +.. option:: StorageClasses + + Semicolon-separated list of names of classes that act as long-lived + storage containers. Default is ``::std::vector``. + +.. option:: StorageFunctions + + Semicolon-separated list of names of member functions that store a + callable into a long-lived container. Default is + ``push_back;emplace_back;insert;assign``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 0eb9e8a243081..aacec4aaa3116 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -114,6 +114,7 @@ Clang-Tidy Checks :doc:`bugprone-infinite-loop <bugprone/infinite-loop>`, :doc:`bugprone-integer-division <bugprone/integer-division>`, :doc:`bugprone-invalid-enum-default-initialization <bugprone/invalid-enum-default-initialization>`, + :doc:`bugprone-lambda-capture-lifetime <bugprone/lambda-capture-lifetime>`, "Yes" :doc:`bugprone-lambda-function-name <bugprone/lambda-function-name>`, :doc:`bugprone-macro-parentheses <bugprone/macro-parentheses>`, "Yes" :doc:`bugprone-macro-repeated-side-effects <bugprone/macro-repeated-side-effects>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-capture-lifetime.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-capture-lifetime.cpp new file mode 100644 index 0000000000000..aa53a7fc02b48 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-capture-lifetime.cpp @@ -0,0 +1,66 @@ +// RUN: %check_clang_tidy %s bugprone-lambda-capture-lifetime %t + +namespace std { +class thread { +public: + template <typename Callable> thread(Callable&& f) {} +}; + +template <typename T> class function { +public: + function() = default; + template <typename Callable> function(Callable&& f) {} +}; + +template <typename T> +class vector { +public: + void emplace_back(T t) {} + void push_back(T t) {} +}; + +template <typename Callable> +void async(Callable&& f) {} +} // namespace std + +std::vector<std::function<int()>> GlobalFns; +std::function<int()> make_function(int); + +void test_thread() { + int x = 0; + + std::thread t1([&x]() { x = 1; }); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: lambda captures local variables by reference, but escapes the local scope + + std::thread t2([x]() { int y = x; }); +} + +void test_async_function() { + int x = 0; + + std::async([&x]() { x = 1; }); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: lambda captures local variables by reference, but escapes the local scope +} + +void test_vector_escape() { + int y = 0; + + GlobalFns.emplace_back([&y]() -> int { return y; }); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: lambda captures local variables by reference, but escapes the local scope + + GlobalFns.push_back(std::function<int()>([&y]() -> int { return y; })); + // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: lambda captures local variables by reference, but escapes the local scope +} + +void test_safe_local_vector() { + int z = 0; + std::vector<std::function<int()>> LocalFns; + + LocalFns.emplace_back([&z]() -> int { return z; }); +} + +void test_nested_lambda_inside_argument_does_not_escape() { + int q = 0; + + GlobalFns.emplace_back(make_function(([&q]() -> int { return q; })())); +} diff --git a/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp b/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp index b193dd3280478..a242ec3594bd8 100644 --- a/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp +++ b/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp @@ -37,6 +37,9 @@ struct RISCVMoveMerge : public MachineFunctionPass { bool isCandidateToMergeMVA01S(const DestSourcePair &RegPair); bool isCandidateToMergeMVSA01(const DestSourcePair &RegPair); + + bool isPLIPairCandidate(const MachineInstr &MI, bool EvenRegPair); + // Merge the two instructions indicated into a single pair instruction. MachineBasicBlock::iterator mergeGPRPairInsns(MachineBasicBlock::iterator I, @@ -44,17 +47,22 @@ struct RISCVMoveMerge : public MachineFunctionPass { MachineBasicBlock::iterator mergePairedInsns(MachineBasicBlock::iterator I, MachineBasicBlock::iterator Paired, bool MoveFromSToA); + MachineBasicBlock::iterator mergePLIPair(MachineBasicBlock::iterator I, + MachineBasicBlock::iterator Paired, + bool RegPairIsEven); MachineBasicBlock::iterator - findMatchingInstPair(MachineBasicBlock::iterator &MBBI, bool EvenRegPair, - const DestSourcePair &RegPair); + findMatchingGPRPairCopy(MachineBasicBlock::iterator &MBBI, bool EvenRegPair, + const DestSourcePair &RegPair); // Look for C.MV instruction that can be combined with // the given instruction into CM.MVA01S or CM.MVSA01. Return the matching // instruction if one exists. MachineBasicBlock::iterator - findMatchingInst(MachineBasicBlock::iterator &MBBI, bool MoveFromSToA, - const DestSourcePair &RegPair); - bool mergeMoveSARegPair(MachineBasicBlock &MBB); + findMatchingSACopy(MachineBasicBlock::iterator &MBBI, bool MoveFromSToA, + const DestSourcePair &RegPair); + MachineBasicBlock::iterator findMatchingPLI(MachineBasicBlock::iterator &MBBI, + bool EvenRegPair); + bool mergeMovePairs(MachineBasicBlock &MBB); bool runOnMachineFunction(MachineFunction &Fn) override; StringRef getPassName() const override { return RISCV_MOVE_MERGE_NAME; } @@ -87,6 +95,20 @@ static unsigned getCM_MVOpcode(const RISCVSubtarget &ST, bool MoveFromSToA) { llvm_unreachable("Unhandled subtarget with paired move."); } +// Returns 0 if Opc has no paired form. +static unsigned getPairedPLIOpcode(unsigned Opc) { + switch (Opc) { + case RISCV::PLI_B: + return RISCV::PLI_DB; + case RISCV::PLI_H: + return RISCV::PLI_DH; + case RISCV::PLUI_H: + return RISCV::PLUI_DH; + default: + return 0; + } +} + bool RISCVMoveMerge::isGPRPairCopyCandidate(const DestSourcePair &RegPair, bool EvenRegPair) { Register Destination = RegPair.Destination->getReg(); @@ -132,6 +154,21 @@ bool RISCVMoveMerge::isCandidateToMergeMVSA01(const DestSourcePair &RegPair) { return false; } +// Check if MI is a single-reg pli/plui whose destination is a half of a +// GPRPair. +bool RISCVMoveMerge::isPLIPairCandidate(const MachineInstr &MI, + bool EvenRegPair) { + if (!ST->hasStdExtP() || ST->is64Bit()) + return false; + if (!getPairedPLIOpcode(MI.getOpcode())) + return false; + unsigned SubIdx = EvenRegPair ? RISCV::sub_gpr_even : RISCV::sub_gpr_odd; + return TRI + ->getMatchingSuperReg(MI.getOperand(0).getReg(), SubIdx, + &RISCV::GPRPairRegClass) + .isValid(); +} + MachineBasicBlock::iterator RISCVMoveMerge::mergeGPRPairInsns(MachineBasicBlock::iterator I, MachineBasicBlock::iterator Paired, @@ -229,9 +266,34 @@ RISCVMoveMerge::mergePairedInsns(MachineBasicBlock::iterator I, } MachineBasicBlock::iterator -RISCVMoveMerge::findMatchingInstPair(MachineBasicBlock::iterator &MBBI, - bool EvenRegPair, - const DestSourcePair &RegPair) { +RISCVMoveMerge::mergePLIPair(MachineBasicBlock::iterator I, + MachineBasicBlock::iterator Paired, + bool RegPairIsEven) { + MachineBasicBlock::iterator E = I->getParent()->end(); + MachineBasicBlock::iterator NextI = next_nodbg(I, E); + + if (NextI == Paired) + NextI = next_nodbg(NextI, E); + DebugLoc DL = I->getDebugLoc(); + + unsigned Opcode = getPairedPLIOpcode(I->getOpcode()); + unsigned GPRPairIdx = + RegPairIsEven ? RISCV::sub_gpr_even : RISCV::sub_gpr_odd; + Register DestReg = TRI->getMatchingSuperReg( + I->getOperand(0).getReg(), GPRPairIdx, &RISCV::GPRPairRegClass); + + BuildMI(*I->getParent(), I, DL, TII->get(Opcode), DestReg) + .addImm(I->getOperand(1).getImm()); + + I->eraseFromParent(); + Paired->eraseFromParent(); + return NextI; +} + +MachineBasicBlock::iterator +RISCVMoveMerge::findMatchingGPRPairCopy(MachineBasicBlock::iterator &MBBI, + bool EvenRegPair, + const DestSourcePair &RegPair) { MachineBasicBlock::iterator E = MBBI->getParent()->end(); ModifiedRegUnits.clear(); UsedRegUnits.clear(); @@ -278,9 +340,9 @@ RISCVMoveMerge::findMatchingInstPair(MachineBasicBlock::iterator &MBBI, } MachineBasicBlock::iterator -RISCVMoveMerge::findMatchingInst(MachineBasicBlock::iterator &MBBI, - bool MoveFromSToA, - const DestSourcePair &RegPair) { +RISCVMoveMerge::findMatchingSACopy(MachineBasicBlock::iterator &MBBI, + bool MoveFromSToA, + const DestSourcePair &RegPair) { MachineBasicBlock::iterator E = MBBI->getParent()->end(); // Track which register units have been modified and used between the first @@ -325,13 +387,72 @@ RISCVMoveMerge::findMatchingInst(MachineBasicBlock::iterator &MBBI, return E; } +// Look for a same-opcode pli/plui writing the other lane of the same GPRPair +// with the same immediate. Return the matching instruction if one exists. +MachineBasicBlock::iterator +RISCVMoveMerge::findMatchingPLI(MachineBasicBlock::iterator &MBBI, + bool EvenRegPair) { + MachineBasicBlock::iterator E = MBBI->getParent()->end(); + ModifiedRegUnits.clear(); + UsedRegUnits.clear(); + unsigned Opc = MBBI->getOpcode(); + Register FirstDestReg = MBBI->getOperand(0).getReg(); + int64_t FirstImm = MBBI->getOperand(1).getImm(); + unsigned RegPairIdx = EvenRegPair ? RISCV::sub_gpr_even : RISCV::sub_gpr_odd; + unsigned SecondPairIdx = + !EvenRegPair ? RISCV::sub_gpr_even : RISCV::sub_gpr_odd; + + // Get the expected destination register of the matching lane. + Register DestGPRPair = TRI->getMatchingSuperReg(FirstDestReg, RegPairIdx, + &RISCV::GPRPairRegClass); + Register ExpectedDestReg = TRI->getSubReg(DestGPRPair, SecondPairIdx); + + for (MachineBasicBlock::iterator I = next_nodbg(MBBI, E); I != E; + I = next_nodbg(I, E)) { + + MachineInstr &MI = *I; + + if (MI.getOpcode() == Opc) { + Register DestReg = MI.getOperand(0).getReg(); + int64_t Imm = MI.getOperand(1).getImm(); + + if (FirstDestReg == DestReg) + return E; + + // Check if the second PLI matches the other lane and immediate. + if (DestReg == ExpectedDestReg && Imm == FirstImm) + return I; + } + // Update modified / used register units. + LiveRegUnits::accumulateUsedDefed(MI, ModifiedRegUnits, UsedRegUnits, TRI); + // Once the expected lane register is clobbered/read in-between, we can + // stop scanning since the pair cannot be legally merged anymore. + if (!ModifiedRegUnits.available(ExpectedDestReg) || + !UsedRegUnits.available(ExpectedDestReg)) + return E; + } + return E; +} + // Finds instructions, which could be represented as C.MV instructions and // merged into CM.MVA01S or CM.MVSA01. -bool RISCVMoveMerge::mergeMoveSARegPair(MachineBasicBlock &MBB) { +bool RISCVMoveMerge::mergeMovePairs(MachineBasicBlock &MBB) { bool Modified = false; for (MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end(); MBBI != E;) { + // Try merging a pair of single-reg PLI/PLUI into a paired form. + bool IsPLIEven = isPLIPairCandidate(*MBBI, /*EvenRegPair=*/true); + bool IsPLIOdd = isPLIPairCandidate(*MBBI, /*EvenRegPair=*/false); + if (IsPLIEven != IsPLIOdd) { + MachineBasicBlock::iterator Paired = findMatchingPLI(MBBI, IsPLIEven); + if (Paired != E) { + MBBI = mergePLIPair(MBBI, Paired, IsPLIEven); + Modified = true; + continue; + } + } + // Check if the instruction can be compressed to C.MV instruction. If it // can, return Dest/Src register pair. auto RegPair = TII->isCopyInstrImpl(*MBBI); @@ -347,7 +468,7 @@ bool RISCVMoveMerge::mergeMoveSARegPair(MachineBasicBlock &MBB) { MachineBasicBlock::iterator Paired = E; if (MoveFromSToA || MoveFromAToS) { - Paired = findMatchingInst(MBBI, MoveFromSToA, *RegPair); + Paired = findMatchingSACopy(MBBI, MoveFromSToA, *RegPair); if (Paired != E) { MBBI = mergePairedInsns(MBBI, Paired, MoveFromSToA); Modified = true; @@ -355,7 +476,7 @@ bool RISCVMoveMerge::mergeMoveSARegPair(MachineBasicBlock &MBB) { } } if (IsEven != IsOdd) { - Paired = findMatchingInstPair(MBBI, IsEven, *RegPair); + Paired = findMatchingGPRPairCopy(MBBI, IsEven, *RegPair); if (Paired != E) { MBBI = mergeGPRPairInsns(MBBI, Paired, IsEven); Modified = true; @@ -387,7 +508,7 @@ bool RISCVMoveMerge::runOnMachineFunction(MachineFunction &Fn) { UsedRegUnits.init(*TRI); bool Modified = false; for (auto &MBB : Fn) - Modified |= mergeMoveSARegPair(MBB); + Modified |= mergeMovePairs(MBB); return Modified; } diff --git a/llvm/test/CodeGen/RISCV/rv32-move-merge.ll b/llvm/test/CodeGen/RISCV/rv32-move-merge.ll index 4a541cbfb4ded..646cee53d4bcf 100644 --- a/llvm/test/CodeGen/RISCV/rv32-move-merge.ll +++ b/llvm/test/CodeGen/RISCV/rv32-move-merge.ll @@ -44,3 +44,97 @@ define i64 @mv_to_fmv(i64 %a, i64 %b) nounwind { call void @foo() ret i64 %1 } + +; RV32 P-ext packed splat constants flow through the ABI as i64, get split +; by SelectionDAG into two i32 halves materialized as single-reg pli/plui; +; MoveMerger folds matching pairs into pli.db/pli.dh/plui.dh. + +define i64 @pli_b_pair() nounwind { +; CHECK32ZDINX-LABEL: pli_b_pair: +; CHECK32ZDINX: # %bb.0: +; CHECK32ZDINX-NEXT: lui a0, 20560 +; CHECK32ZDINX-NEXT: addi a0, a0, 1285 +; CHECK32ZDINX-NEXT: mv a1, a0 +; CHECK32ZDINX-NEXT: ret +; +; CHECK32P-LABEL: pli_b_pair: +; CHECK32P: # %bb.0: +; CHECK32P-NEXT: pli.db a0, 5 +; CHECK32P-NEXT: ret + ret i64 361700864190383365 +} + +define i64 @pli_b_pair_negative() nounwind { +; CHECK32ZDINX-LABEL: pli_b_pair_negative: +; CHECK32ZDINX: # %bb.0: +; CHECK32ZDINX-NEXT: lui a0, 1040352 +; CHECK32ZDINX-NEXT: addi a0, a0, -515 +; CHECK32ZDINX-NEXT: mv a1, a0 +; CHECK32ZDINX-NEXT: ret +; +; CHECK32P-LABEL: pli_b_pair_negative: +; CHECK32P: # %bb.0: +; CHECK32P-NEXT: pli.db a0, -3 +; CHECK32P-NEXT: ret + ret i64 -144680345676153347 +} + +define i64 @pli_h_pair() nounwind { +; CHECK32ZDINX-LABEL: pli_h_pair: +; CHECK32ZDINX: # %bb.0: +; CHECK32ZDINX-NEXT: lui a0, 672 +; CHECK32ZDINX-NEXT: addi a0, a0, 42 +; CHECK32ZDINX-NEXT: mv a1, a0 +; CHECK32ZDINX-NEXT: ret +; +; CHECK32P-LABEL: pli_h_pair: +; CHECK32P: # %bb.0: +; CHECK32P-NEXT: pli.dh a0, 42 +; CHECK32P-NEXT: ret + ret i64 11822129413226538 +} + +define i64 @pli_h_pair_negative() nounwind { +; CHECK32ZDINX-LABEL: pli_h_pair_negative: +; CHECK32ZDINX: # %bb.0: +; CHECK32ZDINX-NEXT: lui a0, 1048512 +; CHECK32ZDINX-NEXT: addi a0, a0, -5 +; CHECK32ZDINX-NEXT: mv a1, a0 +; CHECK32ZDINX-NEXT: ret +; +; CHECK32P-LABEL: pli_h_pair_negative: +; CHECK32P: # %bb.0: +; CHECK32P-NEXT: pli.dh a0, -5 +; CHECK32P-NEXT: ret + ret i64 -1125917086973957 +} + +define i64 @plui_h_pair() nounwind { +; CHECK32ZDINX-LABEL: plui_h_pair: +; CHECK32ZDINX: # %bb.0: +; CHECK32ZDINX-NEXT: lui a0, 221187 +; CHECK32ZDINX-NEXT: addi a0, a0, 1536 +; CHECK32ZDINX-NEXT: mv a1, a0 +; CHECK32ZDINX-NEXT: ret +; +; CHECK32P-LABEL: plui_h_pair: +; CHECK32P: # %bb.0: +; CHECK32P-NEXT: plui.dh a0, 216 +; CHECK32P-NEXT: ret + ret i64 3891169452581991936 +} + +define i64 @plui_h_pair_negative() nounwind { +; CHECK32ZDINX-LABEL: plui_h_pair_negative: +; CHECK32ZDINX: # %bb.0: +; CHECK32ZDINX-NEXT: lui a0, 1032208 +; CHECK32ZDINX-NEXT: addi a0, a0, -1024 +; CHECK32ZDINX-NEXT: mv a1, a0 +; CHECK32ZDINX-NEXT: ret +; +; CHECK32P-LABEL: plui_h_pair_negative: +; CHECK32P: # %bb.0: +; CHECK32P-NEXT: plui.dh a0, -16 +; CHECK32P-NEXT: ret + ret i64 -287953294993589248 +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
