From ffe21f1cd7ddc0a8f5e8f1a4ed137398d3bf0d83 Mon Sep 17 00:00:00 2001 From: Samira Bakon Date: Wed, 20 Aug 2025 11:40:06 -0400 Subject: [PATCH 01/11] [clang][dataflow] Transfer more cast expressions. (#153066) Transfer all casts by kind as we currently do implicit casts. This obviates the need for specific handling of static casts. Also transfer CK_BaseToDerived and CK_DerivedToBase and add tests for these and missing tests for already-handled cast types. Ensure that CK_BaseToDerived casts result in modeling of the fields of the derived class. --- .../Analysis/FlowSensitive/StorageLocation.h | 11 + clang/lib/Analysis/FlowSensitive/Transfer.cpp | 71 ++++- .../Analysis/FlowSensitive/TransferTest.cpp | 280 +++++++++++++++++- 3 files changed, 349 insertions(+), 13 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h index 8fcc6a44027a..534b9a017d8f 100644 --- a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h +++ b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h @@ -17,6 +17,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/Type.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Debug.h" #include @@ -152,6 +153,11 @@ public: return {SyntheticFields.begin(), SyntheticFields.end()}; } + /// Add a synthetic field, if none by that name is already present. + void addSyntheticField(llvm::StringRef Name, StorageLocation &Loc) { + SyntheticFields.insert({Name, &Loc}); + } + /// Changes the child storage location for a field `D` of reference type. /// All other fields cannot change their storage location and always retain /// the storage location passed to the `RecordStorageLocation` constructor. @@ -164,6 +170,11 @@ public: Children[&D] = Loc; } + /// Add a child storage location for a field `D`, if not already present. + void addChild(const ValueDecl &D, StorageLocation *Loc) { + Children.insert({&D, Loc}); + } + llvm::iterator_range children() const { return {Children.begin(), Children.end()}; } diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 86a816e2e406..23a6de45e18b 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -20,14 +20,17 @@ #include "clang/AST/OperationKinds.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtVisitor.h" +#include "clang/AST/Type.h" #include "clang/Analysis/FlowSensitive/ASTOps.h" #include "clang/Analysis/FlowSensitive/AdornedCFG.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/NoopAnalysis.h" #include "clang/Analysis/FlowSensitive/RecordOps.h" +#include "clang/Analysis/FlowSensitive/StorageLocation.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Basic/Builtins.h" +#include "clang/Basic/LLVM.h" #include "clang/Basic/OperatorKinds.h" #include "llvm/Support/Casting.h" #include @@ -287,7 +290,7 @@ public: } } - void VisitImplicitCastExpr(const ImplicitCastExpr *S) { + void VisitCastExpr(const CastExpr *S) { const Expr *SubExpr = S->getSubExpr(); assert(SubExpr != nullptr); @@ -317,6 +320,60 @@ public: break; } + case CK_BaseToDerived: { + // This is a cast of (single-layer) pointer or reference to a record type. + // We should now model the fields for the derived type. + + // Get the RecordStorageLocation for the record object underneath. + RecordStorageLocation *Loc = nullptr; + if (S->getType()->isPointerType()) { + auto *PV = Env.get(*SubExpr); + assert(PV != nullptr); + if (PV == nullptr) + break; + Loc = cast(&PV->getPointeeLoc()); + } else { + assert(S->getType()->isRecordType()); + if (SubExpr->isGLValue()) { + Loc = Env.get(*SubExpr); + } else { + Loc = &Env.getResultObjectLocation(*SubExpr); + } + } + if (!Loc) { + // Nowhere to add children or propagate from, so we're done. + break; + } + + // Get the derived record type underneath the reference or pointer. + QualType Derived = S->getType().getNonReferenceType(); + if (Derived->isPointerType()) { + Derived = Derived->getPointeeType(); + } + + // Add children to the storage location for fields (including synthetic + // fields) of the derived type and initialize their values. + for (const FieldDecl *Field : + Env.getDataflowAnalysisContext().getModeledFields(Derived)) { + assert(Field != nullptr); + QualType FieldType = Field->getType(); + if (FieldType->isReferenceType()) { + Loc->addChild(*Field, nullptr); + } else { + Loc->addChild(*Field, &Env.createStorageLocation(FieldType)); + } + + for (const auto &Entry : + Env.getDataflowAnalysisContext().getSyntheticFields(Derived)) { + Loc->addSyntheticField(Entry.getKey(), + Env.createStorageLocation(Entry.getValue())); + } + } + Env.initializeFieldsWithValues(*Loc, Derived); + + // Fall through to propagate SubExpr's StorageLocation to the CastExpr. + [[fallthrough]]; + } case CK_IntegralCast: // FIXME: This cast creates a new integral value from the // subexpression. But, because we don't model integers, we don't @@ -324,10 +381,9 @@ public: // modeling is added, then update this code to create a fresh location and // value. case CK_UncheckedDerivedToBase: + case CK_DerivedToBase: case CK_ConstructorConversion: case CK_UserDefinedConversion: - // FIXME: Add tests that excercise CK_UncheckedDerivedToBase, - // CK_ConstructorConversion, and CK_UserDefinedConversion. case CK_NoOp: { // FIXME: Consider making `Environment::getStorageLocation` skip noop // expressions (this and other similar expressions in the file) instead @@ -684,15 +740,6 @@ public: propagateValue(*SubExpr, *S, Env); } - void VisitCXXStaticCastExpr(const CXXStaticCastExpr *S) { - if (S->getCastKind() == CK_NoOp) { - const Expr *SubExpr = S->getSubExpr(); - assert(SubExpr != nullptr); - - propagateValueOrStorageLocation(*SubExpr, *S, Env); - } - } - void VisitConditionalOperator(const ConditionalOperator *S) { const Environment *TrueEnv = StmtToEnv.getEnvironment(*S->getTrueExpr()); const Environment *FalseEnv = StmtToEnv.getEnvironment(*S->getFalseExpr()); diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 214aaee9f97f..96e759e73c15 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -9,17 +9,25 @@ #include "TestingSupport.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/OperationKinds.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/NoopAnalysis.h" +#include "clang/Analysis/FlowSensitive/NoopLattice.h" #include "clang/Analysis/FlowSensitive/RecordOps.h" #include "clang/Analysis/FlowSensitive/StorageLocation.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Basic/LangStandard.h" #include "clang/Testing/TestAST.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" #include "llvm/Testing/Support/Error.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -27,6 +35,7 @@ #include #include #include +#include namespace clang { namespace dataflow { @@ -3541,7 +3550,7 @@ TEST(TransferTest, ResultObjectLocationDontVisitUnevaluatedContexts) { testFunction(Code, "noexceptTarget"); } -TEST(TransferTest, StaticCast) { +TEST(TransferTest, StaticCastNoOp) { std::string Code = R"( void target(int Foo) { int Bar = static_cast(Foo); @@ -3561,6 +3570,13 @@ TEST(TransferTest, StaticCast) { const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); ASSERT_THAT(BarDecl, NotNull()); + const auto *Cast = ast_matchers::selectFirst( + "cast", + ast_matchers::match(ast_matchers::cxxStaticCastExpr().bind("cast"), + ASTCtx)); + ASSERT_THAT(Cast, NotNull()); + ASSERT_EQ(Cast->getCastKind(), CK_NoOp); + const auto *FooVal = Env.getValue(*FooDecl); const auto *BarVal = Env.getValue(*BarDecl); EXPECT_TRUE(isa(FooVal)); @@ -3569,6 +3585,268 @@ TEST(TransferTest, StaticCast) { }); } +TEST(TransferTest, StaticCastBaseToDerived) { + std::string Code = R"cc( + struct Base { + char C; + }; + struct Intermediate : public Base { + bool B; + }; + struct Derived : public Intermediate { + int I; + }; + Base& getBaseRef(); + void target(Base* BPtr) { + Derived* DPtr = static_cast(BPtr); + DPtr->C; + DPtr->B; + DPtr->I; + Derived& DRef = static_cast(*BPtr); + DRef.C; + DRef.B; + DRef.I; + Derived& DRefFromFunc = static_cast(getBaseRef()); + DRefFromFunc.C; + DRefFromFunc.B; + DRefFromFunc.I; + // [[p]] + } + )cc"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + const ValueDecl *BPtrDecl = findValueDecl(ASTCtx, "BPtr"); + ASSERT_THAT(BPtrDecl, NotNull()); + + const ValueDecl *DPtrDecl = findValueDecl(ASTCtx, "DPtr"); + ASSERT_THAT(DPtrDecl, NotNull()); + + const ValueDecl *DRefDecl = findValueDecl(ASTCtx, "DRef"); + ASSERT_THAT(DRefDecl, NotNull()); + + const ValueDecl *DRefFromFuncDecl = + findValueDecl(ASTCtx, "DRefFromFunc"); + ASSERT_THAT(DRefFromFuncDecl, NotNull()); + + const auto *Cast = ast_matchers::selectFirst( + "cast", + ast_matchers::match(ast_matchers::cxxStaticCastExpr().bind("cast"), + ASTCtx)); + ASSERT_THAT(Cast, NotNull()); + ASSERT_EQ(Cast->getCastKind(), CK_BaseToDerived); + + EXPECT_EQ(Env.getValue(*BPtrDecl), Env.getValue(*DPtrDecl)); + EXPECT_EQ(&Env.get(*BPtrDecl)->getPointeeLoc(), + Env.getStorageLocation(*DRefDecl)); + // For DRefFromFunc, not crashing when analyzing the field accesses is + // enough. + }); +} + +TEST(TransferTest, ExplicitDerivedToBaseCast) { + std::string Code = R"cc( + struct Base {}; + struct Derived : public Base {}; + void target(Derived D) { + (Base*)&D; + // [[p]] + } +)cc"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + auto *Cast = ast_matchers::selectFirst( + "cast", ast_matchers::match( + ast_matchers::implicitCastExpr().bind("cast"), ASTCtx)); + ASSERT_THAT(Cast, NotNull()); + ASSERT_EQ(Cast->getCastKind(), CK_DerivedToBase); + + auto *AddressOf = ast_matchers::selectFirst( + "addressof", + ast_matchers::match(ast_matchers::unaryOperator().bind("addressof"), + ASTCtx)); + ASSERT_THAT(AddressOf, NotNull()); + ASSERT_EQ(AddressOf->getOpcode(), UO_AddrOf); + + EXPECT_EQ(Env.getValue(*Cast), Env.getValue(*AddressOf)); + }); +} + +TEST(TransferTest, ConstructorConversion) { + std::string Code = R"cc( + struct Base {}; + struct Derived : public Base {}; + void target(Derived D) { + Base B = (Base)D; + // [[p]] + } +)cc"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + auto *Cast = ast_matchers::selectFirst( + "cast", ast_matchers::match( + ast_matchers::cStyleCastExpr().bind("cast"), ASTCtx)); + ASSERT_THAT(Cast, NotNull()); + ASSERT_EQ(Cast->getCastKind(), CK_ConstructorConversion); + + auto &DLoc = getLocForDecl(ASTCtx, Env, "D"); + auto &BLoc = getLocForDecl(ASTCtx, Env, "B"); + EXPECT_NE(&BLoc, &DLoc); + }); +} + +TEST(TransferTest, UserDefinedConversion) { + std::string Code = R"cc( + struct To {}; + struct From { + operator To(); + }; + void target(From F) { + To T = (To)F; + // [[p]] + } +)cc"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + auto *Cast = ast_matchers::selectFirst( + "cast", ast_matchers::match( + ast_matchers::implicitCastExpr().bind("cast"), ASTCtx)); + ASSERT_THAT(Cast, NotNull()); + ASSERT_EQ(Cast->getCastKind(), CK_UserDefinedConversion); + + auto &FLoc = getLocForDecl(ASTCtx, Env, "F"); + auto &TLoc = getLocForDecl(ASTCtx, Env, "T"); + EXPECT_NE(&TLoc, &FLoc); + }); +} + +TEST(TransferTest, ImplicitUncheckedDerivedToBaseCast) { + std::string Code = R"cc( + struct Base { + void method(); + }; + struct Derived : public Base {}; + void target(Derived D) { + D.method(); + // [[p]] + } +)cc"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + auto *Cast = ast_matchers::selectFirst( + "cast", ast_matchers::match( + ast_matchers::implicitCastExpr().bind("cast"), ASTCtx)); + ASSERT_THAT(Cast, NotNull()); + ASSERT_EQ(Cast->getCastKind(), CK_UncheckedDerivedToBase); + + auto &DLoc = getLocForDecl(ASTCtx, Env, "D"); + EXPECT_EQ(Env.getStorageLocation(*Cast), &DLoc); + }); +} + +TEST(TransferTest, ImplicitDerivedToBaseCast) { + std::string Code = R"cc( + struct Base {}; + struct Derived : public Base {}; + void target() { + Base* B = new Derived(); + // [[p]] + } +)cc"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + auto *Cast = ast_matchers::selectFirst( + "cast", ast_matchers::match( + ast_matchers::implicitCastExpr().bind("cast"), ASTCtx)); + ASSERT_THAT(Cast, NotNull()); + ASSERT_EQ(Cast->getCastKind(), CK_DerivedToBase); + + auto *New = ast_matchers::selectFirst( + "new", ast_matchers::match(ast_matchers::cxxNewExpr().bind("new"), + ASTCtx)); + ASSERT_THAT(New, NotNull()); + + EXPECT_EQ(Env.getValue(*Cast), Env.getValue(*New)); + }); +} + +TEST(TransferTest, ReinterpretCast) { + std::string Code = R"cc( + struct S { + int I; + }; + + void target(unsigned char* Bytes) { + S& SRef = reinterpret_cast(Bytes); + SRef.I; + S* SPtr = reinterpret_cast(Bytes); + SPtr->I; + // [[p]] + } + )cc"; + runDataflow(Code, [](const llvm::StringMap> + &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + const ValueDecl *I = findValueDecl(ASTCtx, "I"); + ASSERT_THAT(I, NotNull()); + + // No particular knowledge of I's value is modeled, but for both casts, + // the fields of S are modeled. + + { + auto &Loc = getLocForDecl(ASTCtx, Env, "SRef"); + std::vector Children; + for (const auto &Entry : Loc.children()) { + Children.push_back(Entry.getFirst()); + } + + EXPECT_THAT(Children, UnorderedElementsAre(I)); + } + + { + auto &Loc = cast( + getValueForDecl(ASTCtx, Env, "SPtr").getPointeeLoc()); + std::vector Children; + for (const auto &Entry : Loc.children()) { + Children.push_back(Entry.getFirst()); + } + + EXPECT_THAT(Children, UnorderedElementsAre(I)); + } + }); +} + TEST(TransferTest, IntegralCast) { std::string Code = R"( void target(int Foo) { From 15cb06109de8ccb222c215e88c16ac6f7fa3ba0e Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 20 Aug 2025 10:56:30 -0500 Subject: [PATCH 02/11] [Frontend][OpenMP] Allow multiple occurrences of DYN_GROUPPRIVATE (#154549) It was mistakenly placed in "allowOnceClauses" on the constructs that allow it. --- llvm/include/llvm/Frontend/OpenMP/OMP.td | 56 +++++++++++------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td index 202f684d808b..8d62f7422a9d 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.td +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td @@ -1103,6 +1103,7 @@ def OMP_Target : Directive<[Spelling<"target">]> { let allowedClauses = [ VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -1115,7 +1116,6 @@ def OMP_Target : Directive<[Spelling<"target">]> { let allowedOnceClauses = [ VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -1258,6 +1258,7 @@ def OMP_TaskYield : Directive<[Spelling<"taskyield">]> { def OMP_Teams : Directive<[Spelling<"teams">]> { let allowedClauses = [ VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -1266,7 +1267,6 @@ def OMP_Teams : Directive<[Spelling<"teams">]> { ]; let allowedOnceClauses = [ VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -1520,6 +1520,7 @@ def OMP_target_loop : Directive<[Spelling<"target loop">]> { let allowedClauses = [ VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -1535,7 +1536,6 @@ def OMP_target_loop : Directive<[Spelling<"target loop">]> { let allowedOnceClauses = [ VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -1982,6 +1982,7 @@ def OMP_TargetParallel : Directive<[Spelling<"target parallel">]> { VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -1997,7 +1998,6 @@ def OMP_TargetParallel : Directive<[Spelling<"target parallel">]> { let allowedOnceClauses = [ VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2011,6 +2011,7 @@ def OMP_TargetParallelDo : Directive<[Spelling<"target parallel do">]> { VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2027,7 +2028,6 @@ def OMP_TargetParallelDo : Directive<[Spelling<"target parallel do">]> { VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2049,6 +2049,7 @@ def OMP_TargetParallelDoSimd VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2070,9 +2071,6 @@ def OMP_TargetParallelDoSimd VersionedClause, VersionedClause, ]; - let allowedOnceClauses = [ - VersionedClause, - ]; let leafConstructs = [OMP_Target, OMP_Parallel, OMP_Do, OMP_Simd]; let category = CA_Executable; let languages = [L_Fortran]; @@ -2085,6 +2083,7 @@ def OMP_TargetParallelFor : Directive<[Spelling<"target parallel for">]> { VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2105,7 +2104,6 @@ def OMP_TargetParallelFor : Directive<[Spelling<"target parallel for">]> { VersionedClause, ]; let allowedOnceClauses = [ - VersionedClause, VersionedClause, VersionedClause, ]; @@ -2123,6 +2121,7 @@ def OMP_TargetParallelForSimd VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2146,7 +2145,6 @@ def OMP_TargetParallelForSimd VersionedClause, ]; let allowedOnceClauses = [ - VersionedClause, VersionedClause, VersionedClause, ]; @@ -2159,6 +2157,7 @@ def OMP_target_parallel_loop : Directive<[Spelling<"target parallel loop">]> { VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2176,7 +2175,6 @@ def OMP_target_parallel_loop : Directive<[Spelling<"target parallel loop">]> { VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2192,6 +2190,7 @@ def OMP_TargetSimd : Directive<[Spelling<"target simd">]> { VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2211,7 +2210,6 @@ def OMP_TargetSimd : Directive<[Spelling<"target simd">]> { VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2228,6 +2226,7 @@ def OMP_TargetTeams : Directive<[Spelling<"target teams">]> { let allowedClauses = [ VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2243,7 +2242,6 @@ def OMP_TargetTeams : Directive<[Spelling<"target teams">]> { VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2258,6 +2256,7 @@ def OMP_TargetTeamsDistribute let allowedClauses = [ VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2276,7 +2275,6 @@ def OMP_TargetTeamsDistribute VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2291,6 +2289,7 @@ def OMP_TargetTeamsDistributeParallelDo let allowedClauses = [ VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2309,7 +2308,6 @@ def OMP_TargetTeamsDistributeParallelDo VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2329,6 +2327,7 @@ def OMP_TargetTeamsDistributeParallelDoSimd VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2348,7 +2347,6 @@ def OMP_TargetTeamsDistributeParallelDoSimd VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2374,6 +2372,7 @@ def OMP_TargetTeamsDistributeParallelFor VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2394,7 +2393,6 @@ def OMP_TargetTeamsDistributeParallelFor VersionedClause, ]; let allowedOnceClauses = [ - VersionedClause, VersionedClause, ]; let leafConstructs = @@ -2413,6 +2411,7 @@ def OMP_TargetTeamsDistributeParallelForSimd VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2437,7 +2436,6 @@ def OMP_TargetTeamsDistributeParallelForSimd VersionedClause, ]; let allowedOnceClauses = [ - VersionedClause, VersionedClause, ]; let leafConstructs = @@ -2451,6 +2449,7 @@ def OMP_TargetTeamsDistributeSimd VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2470,7 +2469,6 @@ def OMP_TargetTeamsDistributeSimd VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2488,6 +2486,7 @@ def OMP_target_teams_loop : Directive<[Spelling<"target teams loop">]> { VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2504,7 +2503,6 @@ def OMP_target_teams_loop : Directive<[Spelling<"target teams loop">]> { VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2553,6 +2551,7 @@ def OMP_TeamsDistribute : Directive<[Spelling<"teams distribute">]> { VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2563,7 +2562,6 @@ def OMP_TeamsDistribute : Directive<[Spelling<"teams distribute">]> { VersionedClause, ]; let allowedOnceClauses = [ - VersionedClause, VersionedClause, VersionedClause, ]; @@ -2575,6 +2573,7 @@ def OMP_TeamsDistributeParallelDo let allowedClauses = [ VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2587,7 +2586,6 @@ def OMP_TeamsDistributeParallelDo VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2604,6 +2602,7 @@ def OMP_TeamsDistributeParallelDoSimd let allowedClauses = [ VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2617,7 +2616,6 @@ def OMP_TeamsDistributeParallelDoSimd VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2640,6 +2638,7 @@ def OMP_TeamsDistributeParallelFor VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2654,9 +2653,6 @@ def OMP_TeamsDistributeParallelFor VersionedClause, VersionedClause, ]; - let allowedOnceClauses = [ - VersionedClause, - ]; let leafConstructs = [OMP_Teams, OMP_Distribute, OMP_Parallel, OMP_For]; let category = CA_Executable; let languages = [L_C]; @@ -2669,6 +2665,7 @@ def OMP_TeamsDistributeParallelForSimd VersionedClause, VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2687,9 +2684,6 @@ def OMP_TeamsDistributeParallelForSimd VersionedClause, VersionedClause, ]; - let allowedOnceClauses = [ - VersionedClause, - ]; let leafConstructs = [OMP_Teams, OMP_Distribute, OMP_Parallel, OMP_For, OMP_Simd]; let category = CA_Executable; @@ -2699,6 +2693,7 @@ def OMP_TeamsDistributeSimd : Directive<[Spelling<"teams distribute simd">]> { let allowedClauses = [ VersionedClause, VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2713,7 +2708,6 @@ def OMP_TeamsDistributeSimd : Directive<[Spelling<"teams distribute simd">]> { VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2726,6 +2720,7 @@ def OMP_TeamsDistributeSimd : Directive<[Spelling<"teams distribute simd">]> { def OMP_teams_loop : Directive<[Spelling<"teams loop">]> { let allowedClauses = [ VersionedClause, + VersionedClause, VersionedClause, VersionedClause, VersionedClause, @@ -2737,7 +2732,6 @@ def OMP_teams_loop : Directive<[Spelling<"teams loop">]> { VersionedClause, VersionedClause, VersionedClause, - VersionedClause, VersionedClause, VersionedClause, VersionedClause, From a6da68ed36d7ecb9edf00262d2a2c1129689399f Mon Sep 17 00:00:00 2001 From: Yanzuo Liu Date: Wed, 20 Aug 2025 23:57:30 +0800 Subject: [PATCH 03/11] [Clang][ASTMatchers] Make `hasConditionVariableStatement` support `for` loop, `while` loop and `switch` statements (#154298) Co-authored-by: Aaron Ballman --- clang/docs/ReleaseNotes.rst | 3 ++ clang/include/clang/ASTMatchers/ASTMatchers.h | 17 ++++++---- .../ASTMatchers/ASTMatchersNodeTest.cpp | 33 +++++++++++++++++++ .../ASTMatchers/ASTMatchersTraversalTest.cpp | 15 --------- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 6d60b03d9f62..98a6f209fe2d 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -329,6 +329,9 @@ AST Matchers - Add a boolean member ``IgnoreSystemHeaders`` to ``MatchFinderOptions``. This allows it to ignore nodes in system headers when traversing the AST. +- ``hasConditionVariableStatement`` now supports ``for`` loop, ``while`` loop + and ``switch`` statements. + clang-format ------------ - Add ``SpaceInEmptyBraces`` option and set it to ``Always`` for WebKit style. diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h index 289711ddfb1b..4520c4af38c8 100644 --- a/clang/include/clang/ASTMatchers/ASTMatchers.h +++ b/clang/include/clang/ASTMatchers/ASTMatchers.h @@ -5661,8 +5661,8 @@ AST_POLYMORPHIC_MATCHER_P(hasInitStatement, return Init != nullptr && InnerMatcher.matches(*Init, Finder, Builder); } -/// Matches the condition expression of an if statement, for loop, -/// switch statement or conditional operator. +/// Matches the condition expression of an if statement, for loop, while loop, +/// do-while loop, switch statement or conditional operator. /// /// Example matches true (matcher = hasCondition(cxxBoolLiteral(equals(true)))) /// \code @@ -5747,16 +5747,21 @@ AST_MATCHER_P(Decl, declaresSameEntityAsBoundNode, std::string, ID) { }); } -/// Matches the condition variable statement in an if statement. +/// Matches the condition variable statement in an if statement, for loop, +/// while loop or switch statement. /// /// Given /// \code /// if (A* a = GetAPointer()) {} +/// for (; A* a = GetAPointer(); ) {} /// \endcode /// hasConditionVariableStatement(...) -/// matches 'A* a = GetAPointer()'. -AST_MATCHER_P(IfStmt, hasConditionVariableStatement, - internal::Matcher, InnerMatcher) { +/// matches both 'A* a = GetAPointer()'. +AST_POLYMORPHIC_MATCHER_P(hasConditionVariableStatement, + AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, ForStmt, + WhileStmt, + SwitchStmt), + internal::Matcher, InnerMatcher) { const DeclStmt* const DeclarationStatement = Node.getConditionVariableDeclStmt(); return DeclarationStatement != nullptr && diff --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp index b55928f7060d..d7df9cae01f3 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp @@ -1183,6 +1183,39 @@ TEST_P(ASTMatchersTest, AsmStatement) { EXPECT_TRUE(matches("void foo() { __asm(\"mov al, 2\"); }", asmStmt())); } +TEST_P(ASTMatchersTest, HasConditionVariableStatement) { + if (!GetParam().isCXX()) { + // FIXME: Add a test for `hasConditionVariableStatement()` that does not + // depend on C++. + return; + } + + StatementMatcher IfCondition = + ifStmt(hasConditionVariableStatement(declStmt())); + + EXPECT_TRUE(matches("void x() { if (int* a = 0) {} }", IfCondition)); + EXPECT_TRUE(notMatches("void x() { if (true) {} }", IfCondition)); + EXPECT_TRUE(notMatches("void x() { int x; if ((x = 42)) {} }", IfCondition)); + + StatementMatcher SwitchCondition = + switchStmt(hasConditionVariableStatement(declStmt())); + + EXPECT_TRUE(matches("void x() { switch (int a = 0) {} }", SwitchCondition)); + if (GetParam().isCXX17OrLater()) { + EXPECT_TRUE( + notMatches("void x() { switch (int a = 0; a) {} }", SwitchCondition)); + } + + StatementMatcher ForCondition = + forStmt(hasConditionVariableStatement(declStmt())); + + EXPECT_TRUE(matches("void x() { for (; int a = 0; ) {} }", ForCondition)); + EXPECT_TRUE(notMatches("void x() { for (int a = 0; ; ) {} }", ForCondition)); + + EXPECT_TRUE(matches("void x() { while (int a = 0) {} }", + whileStmt(hasConditionVariableStatement(declStmt())))); +} + TEST_P(ASTMatchersTest, HasCondition) { if (!GetParam().isCXX()) { // FIXME: Add a test for `hasCondition()` that does not depend on C++. diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp index d58bc00c995e..c0a03deb5b54 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp @@ -5142,21 +5142,6 @@ TEST(ForEachLambdaCapture, MatchExplicitCapturesOnly) { matcher, std::make_unique>("LC", 1))); } -TEST(HasConditionVariableStatement, DoesNotMatchCondition) { - EXPECT_TRUE(notMatches( - "void x() { if(true) {} }", - ifStmt(hasConditionVariableStatement(declStmt())))); - EXPECT_TRUE(notMatches( - "void x() { int x; if((x = 42)) {} }", - ifStmt(hasConditionVariableStatement(declStmt())))); -} - -TEST(HasConditionVariableStatement, MatchesConditionVariables) { - EXPECT_TRUE(matches( - "void x() { if(int* a = 0) {} }", - ifStmt(hasConditionVariableStatement(declStmt())))); -} - TEST(ForEach, BindsOneNode) { EXPECT_TRUE(matchAndVerifyResultTrue("class C { int x; };", recordDecl(hasName("C"), forEach(fieldDecl(hasName("x")).bind("x"))), From f487c0e63cee9aba57335b1c509f7780de849f65 Mon Sep 17 00:00:00 2001 From: Kazu Hirata Date: Wed, 20 Aug 2025 08:58:59 -0700 Subject: [PATCH 04/11] [AST] Fix warnings This patch fixes: clang/lib/AST/ByteCode/InterpBuiltin.cpp:1827:21: error: unused variable 'ASTCtx' [-Werror,-Wunused-variable] clang/lib/AST/ByteCode/InterpBuiltin.cpp:2724:18: error: unused variable 'Arg2Type' [-Werror,-Wunused-variable] clang/lib/AST/ByteCode/InterpBuiltin.cpp:2725:18: error: unused variable 'Arg3Type' [-Werror,-Wunused-variable] clang/lib/AST/ByteCode/InterpBuiltin.cpp:2748:18: error: unused variable 'ElemT' [-Werror,-Wunused-variable] --- clang/lib/AST/ByteCode/InterpBuiltin.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp index 5de5091178b8..93661d8e2ddf 100644 --- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp +++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp @@ -1830,6 +1830,7 @@ static bool interp__builtin_elementwise_countzeroes(InterpState &S, assert(Call->getArg(1)->getType()->isVectorType() && ASTCtx.hasSameUnqualifiedType(Call->getArg(0)->getType(), Call->getArg(1)->getType())); + (void)ASTCtx; ZeroArg = S.Stk.pop(); assert(ZeroArg.getFieldDesc()->isPrimitiveArray()); } @@ -2728,6 +2729,8 @@ static bool interp__builtin_elementwise_fma(InterpState &S, CodePtr OpPC, if (!Arg1Type->isVectorType()) { assert(!Arg2Type->isVectorType()); assert(!Arg3Type->isVectorType()); + (void)Arg2Type; + (void)Arg3Type; const Floating &Z = S.Stk.pop(); const Floating &Y = S.Stk.pop(); @@ -2753,6 +2756,7 @@ static bool interp__builtin_elementwise_fma(InterpState &S, CodePtr OpPC, assert(NumElems == Arg2Type->castAs()->getNumElements() && NumElems == Arg3Type->castAs()->getNumElements()); assert(ElemT->isRealFloatingType()); + (void)ElemT; const Pointer &VZ = S.Stk.pop(); const Pointer &VY = S.Stk.pop(); From 6a285cc8e6c1dd814d5e424f02b5574d2a3e72db Mon Sep 17 00:00:00 2001 From: Matthias Springer Date: Wed, 20 Aug 2025 18:02:24 +0200 Subject: [PATCH 05/11] [mlir][IR] Fix `Block::without_terminator` for blocks without terminator (#154498) Blocks without a terminator are not handled correctly by `Block::without_terminator`: the last operation is excluded, even when it is not a terminator. With this commit, only terminators are excluded. If the last operation is unregistered, it is included for safety. --- mlir/include/mlir/IR/Block.h | 14 ++++++++++---- mlir/lib/Analysis/TopologicalSortUtils.cpp | 7 +------ mlir/lib/IR/Block.cpp | 10 ++++++++++ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/mlir/include/mlir/IR/Block.h b/mlir/include/mlir/IR/Block.h index e486bb627474..85ce66f69df4 100644 --- a/mlir/include/mlir/IR/Block.h +++ b/mlir/include/mlir/IR/Block.h @@ -205,12 +205,14 @@ public: } /// Return an iterator range over the operation within this block excluding - /// the terminator operation at the end. + /// the terminator operation at the end. If the block has no terminator, + /// return an iterator range over the entire block. If it is unknown if the + /// block has a terminator (i.e., last block operation is unregistered), also + /// return an iterator range over the entire block. iterator_range without_terminator() { if (begin() == end()) return {begin(), end()}; - auto endIt = --end(); - return {begin(), endIt}; + return without_terminator_impl(); } //===--------------------------------------------------------------------===// @@ -221,7 +223,8 @@ public: /// the block might have a valid terminator operation. Operation *getTerminator(); - /// Check whether this block might have a terminator. + /// Return "true" if this block might have a terminator. Return "true" if + /// the last operation is unregistered. bool mightHaveTerminator(); //===--------------------------------------------------------------------===// @@ -402,6 +405,9 @@ public: void printAsOperand(raw_ostream &os, AsmState &state); private: + /// Same as `without_terminator`, but assumes that the block is not empty. + iterator_range without_terminator_impl(); + /// Pair of the parent object that owns this block and a bit that signifies if /// the operations within this block have a valid ordering. llvm::PointerIntPair parentValidOpOrderPair; diff --git a/mlir/lib/Analysis/TopologicalSortUtils.cpp b/mlir/lib/Analysis/TopologicalSortUtils.cpp index a2fd14910892..99546e71533a 100644 --- a/mlir/lib/Analysis/TopologicalSortUtils.cpp +++ b/mlir/lib/Analysis/TopologicalSortUtils.cpp @@ -101,12 +101,7 @@ bool mlir::sortTopologically( bool mlir::sortTopologically( Block *block, function_ref isOperandReady) { - if (block->empty()) - return true; - if (block->back().hasTrait()) - return sortTopologically(block, block->without_terminator(), - isOperandReady); - return sortTopologically(block, *block, isOperandReady); + return sortTopologically(block, block->without_terminator(), isOperandReady); } bool mlir::computeTopologicalSorting( diff --git a/mlir/lib/IR/Block.cpp b/mlir/lib/IR/Block.cpp index 57825d9b4217..27b47e2d2653 100644 --- a/mlir/lib/IR/Block.cpp +++ b/mlir/lib/IR/Block.cpp @@ -251,6 +251,16 @@ bool Block::mightHaveTerminator() { return !empty() && back().mightHaveTrait(); } +iterator_range Block::without_terminator_impl() { + // Note: When the op is unregistered, we do not know for sure if the last + // op is a terminator. In that case, we include it in `without_terminator`, + // but that decision is somewhat arbitrary. + if (!back().hasTrait()) + return {begin(), end()}; + auto endIt = --end(); + return {begin(), endIt}; +} + // Indexed successor access. unsigned Block::getNumSuccessors() { return empty() ? 0 : back().getNumSuccessors(); From 562e0211032767a9f2d24d9d8d7868385e948937 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 20 Aug 2025 09:07:58 -0700 Subject: [PATCH 06/11] [RISCV] Minor refactor of RISCVMoveMerge::mergePairedInsns. (#154467) Fold the ARegInFirstPair into the later if/else with the same condition. Use std::swap so we don't need to repeat operands in the opposite order. --- llvm/lib/Target/RISCV/RISCVMoveMerger.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp b/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp index ae311db443bf..efea1b422d58 100644 --- a/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp +++ b/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp @@ -110,8 +110,6 @@ RISCVMoveMerge::mergePairedInsns(MachineBasicBlock::iterator I, MachineBasicBlock::iterator NextI = next_nodbg(I, E); DestSourcePair FirstPair = TII->isCopyInstrImpl(*I).value(); DestSourcePair PairedRegs = TII->isCopyInstrImpl(*Paired).value(); - Register ARegInFirstPair = MoveFromSToA ? FirstPair.Destination->getReg() - : FirstPair.Source->getReg(); if (NextI == Paired) NextI = next_nodbg(NextI, E); @@ -130,7 +128,6 @@ RISCVMoveMerge::mergePairedInsns(MachineBasicBlock::iterator I, // // mv a0, s2 // mv a1, s1 => cm.mva01s s2,s1 - bool StartWithX10 = ARegInFirstPair == RISCV::X10; unsigned Opcode; if (MoveFromSToA) { // We are moving one of the copies earlier so its kill flag may become @@ -141,12 +138,16 @@ RISCVMoveMerge::mergePairedInsns(MachineBasicBlock::iterator I, PairedSource.setIsKill(false); Opcode = getMoveFromSToAOpcode(*ST); - Sreg1 = StartWithX10 ? FirstPair.Source : &PairedSource; - Sreg2 = StartWithX10 ? &PairedSource : FirstPair.Source; + Sreg1 = FirstPair.Source; + Sreg2 = &PairedSource; + if (FirstPair.Destination->getReg() != RISCV::X10) + std::swap(Sreg1, Sreg2); } else { Opcode = getMoveFromAToSOpcode(*ST); - Sreg1 = StartWithX10 ? FirstPair.Destination : PairedRegs.Destination; - Sreg2 = StartWithX10 ? PairedRegs.Destination : FirstPair.Destination; + Sreg1 = FirstPair.Destination; + Sreg2 = PairedRegs.Destination; + if (FirstPair.Source->getReg() != RISCV::X10) + std::swap(Sreg1, Sreg2); } BuildMI(*I->getParent(), I, DL, TII->get(Opcode)).add(*Sreg1).add(*Sreg2); From 2b7b8bdc165a1f9fa933fe531d6a5b152d066297 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 20 Aug 2025 09:09:55 -0700 Subject: [PATCH 07/11] [X86] Accept the canonical form of a sign bit test in MatchVectorAllEqualTest. (#154421) This function tries to look for (seteq (and (reduce_or), mask), 0). If the mask is a sign bit, InstCombine will have turned it into (setgt (reduce_or), -1). We should handle that case too. I'm looking into adding the same canonicalization to SimplifySetCC and this change is needed to prevent test regressions. --- llvm/lib/Target/X86/X86ISelLowering.cpp | 78 ++++++------ llvm/test/CodeGen/X86/vector-reduce-or-cmp.ll | 113 ++++++++++++++++-- 2 files changed, 145 insertions(+), 46 deletions(-) diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp index 2c726a9f7f6c..19131fbd4102 100644 --- a/llvm/lib/Target/X86/X86ISelLowering.cpp +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp @@ -23185,43 +23185,51 @@ static SDValue LowerVectorAllEqual(const SDLoc &DL, SDValue LHS, SDValue RHS, // Check whether an AND/OR'd reduction tree is PTEST-able, or if we can fallback // to CMP(MOVMSK(PCMPEQB(X,Y))). -static SDValue MatchVectorAllEqualTest(SDValue LHS, SDValue RHS, +static SDValue MatchVectorAllEqualTest(SDValue OrigLHS, SDValue OrigRHS, ISD::CondCode CC, const SDLoc &DL, const X86Subtarget &Subtarget, SelectionDAG &DAG, X86::CondCode &X86CC) { - assert((CC == ISD::SETEQ || CC == ISD::SETNE) && "Unsupported ISD::CondCode"); + SDValue Op = OrigLHS; - bool CmpNull = isNullConstant(RHS); - bool CmpAllOnes = isAllOnesConstant(RHS); - if (!CmpNull && !CmpAllOnes) - return SDValue(); + bool CmpNull; + APInt Mask; + if (CC == ISD::SETEQ || CC == ISD::SETNE) { + CmpNull = isNullConstant(OrigRHS); + if (!CmpNull && !isAllOnesConstant(OrigRHS)) + return SDValue(); - SDValue Op = LHS; - if (!Subtarget.hasSSE2() || !Op->hasOneUse()) - return SDValue(); + if (!Subtarget.hasSSE2() || !Op->hasOneUse()) + return SDValue(); - // Check whether we're masking/truncating an OR-reduction result, in which - // case track the masked bits. - // TODO: Add CmpAllOnes support. - APInt Mask = APInt::getAllOnes(Op.getScalarValueSizeInBits()); - if (CmpNull) { - switch (Op.getOpcode()) { - case ISD::TRUNCATE: { - SDValue Src = Op.getOperand(0); - Mask = APInt::getLowBitsSet(Src.getScalarValueSizeInBits(), - Op.getScalarValueSizeInBits()); - Op = Src; - break; - } - case ISD::AND: { - if (auto *Cst = dyn_cast(Op.getOperand(1))) { - Mask = Cst->getAPIntValue(); - Op = Op.getOperand(0); + // Check whether we're masking/truncating an OR-reduction result, in which + // case track the masked bits. + // TODO: Add CmpAllOnes support. + Mask = APInt::getAllOnes(Op.getScalarValueSizeInBits()); + if (CmpNull) { + switch (Op.getOpcode()) { + case ISD::TRUNCATE: { + SDValue Src = Op.getOperand(0); + Mask = APInt::getLowBitsSet(Src.getScalarValueSizeInBits(), + Op.getScalarValueSizeInBits()); + Op = Src; + break; + } + case ISD::AND: { + if (auto *Cst = dyn_cast(Op.getOperand(1))) { + Mask = Cst->getAPIntValue(); + Op = Op.getOperand(0); + } + break; + } } - break; - } } + } else if (CC == ISD::SETGT && isAllOnesConstant(OrigRHS)) { + CC = ISD::SETEQ; + CmpNull = true; + Mask = APInt::getSignMask(Op.getScalarValueSizeInBits()); + } else { + return SDValue(); } ISD::NodeType LogicOp = CmpNull ? ISD::OR : ISD::AND; @@ -56274,14 +56282,16 @@ static SDValue combineSetCC(SDNode *N, SelectionDAG &DAG, if (SDValue V = combineVectorSizedSetCCEquality(VT, LHS, RHS, CC, DL, DAG, Subtarget)) return V; + } - if (VT == MVT::i1) { - X86::CondCode X86CC; - if (SDValue V = - MatchVectorAllEqualTest(LHS, RHS, CC, DL, Subtarget, DAG, X86CC)) - return DAG.getNode(ISD::TRUNCATE, DL, VT, getSETCC(X86CC, V, DL, DAG)); - } + if (VT == MVT::i1) { + X86::CondCode X86CC; + if (SDValue V = + MatchVectorAllEqualTest(LHS, RHS, CC, DL, Subtarget, DAG, X86CC)) + return DAG.getNode(ISD::TRUNCATE, DL, VT, getSETCC(X86CC, V, DL, DAG)); + } + if (CC == ISD::SETNE || CC == ISD::SETEQ) { if (OpVT.isScalarInteger()) { // cmpeq(or(X,Y),X) --> cmpeq(and(~X,Y),0) // cmpne(or(X,Y),X) --> cmpne(and(~X,Y),0) diff --git a/llvm/test/CodeGen/X86/vector-reduce-or-cmp.ll b/llvm/test/CodeGen/X86/vector-reduce-or-cmp.ll index 9cd0f4d12e15..227e000c6be7 100644 --- a/llvm/test/CodeGen/X86/vector-reduce-or-cmp.ll +++ b/llvm/test/CodeGen/X86/vector-reduce-or-cmp.ll @@ -903,6 +903,95 @@ define i1 @mask_v8i32(<8 x i32> %a0) { ret i1 %3 } +define i1 @mask_v8i32_2(<8 x i32> %a0) { +; SSE2-LABEL: mask_v8i32_2: +; SSE2: # %bb.0: +; SSE2-NEXT: por %xmm1, %xmm0 +; SSE2-NEXT: pslld $1, %xmm0 +; SSE2-NEXT: movmskps %xmm0, %eax +; SSE2-NEXT: testl %eax, %eax +; SSE2-NEXT: sete %al +; SSE2-NEXT: retq +; +; SSE41-LABEL: mask_v8i32_2: +; SSE41: # %bb.0: +; SSE41-NEXT: por %xmm1, %xmm0 +; SSE41-NEXT: ptest {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0 +; SSE41-NEXT: sete %al +; SSE41-NEXT: retq +; +; AVX1-LABEL: mask_v8i32_2: +; AVX1: # %bb.0: +; AVX1-NEXT: vptest {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %ymm0 +; AVX1-NEXT: sete %al +; AVX1-NEXT: vzeroupper +; AVX1-NEXT: retq +; +; AVX2-LABEL: mask_v8i32_2: +; AVX2: # %bb.0: +; AVX2-NEXT: vpbroadcastq {{.*#+}} ymm1 = [4611686019501129728,4611686019501129728,4611686019501129728,4611686019501129728] +; AVX2-NEXT: vptest %ymm1, %ymm0 +; AVX2-NEXT: sete %al +; AVX2-NEXT: vzeroupper +; AVX2-NEXT: retq +; +; AVX512-LABEL: mask_v8i32_2: +; AVX512: # %bb.0: +; AVX512-NEXT: vpbroadcastq {{.*#+}} ymm1 = [4611686019501129728,4611686019501129728,4611686019501129728,4611686019501129728] +; AVX512-NEXT: vptest %ymm1, %ymm0 +; AVX512-NEXT: sete %al +; AVX512-NEXT: vzeroupper +; AVX512-NEXT: retq + %1 = call i32 @llvm.vector.reduce.or.v8i32(<8 x i32> %a0) + %2 = and i32 %1, 1073741824 + %3 = icmp eq i32 %2, 0 + ret i1 %3 +} + + +define i1 @signtest_v8i32(<8 x i32> %a0) { +; SSE2-LABEL: signtest_v8i32: +; SSE2: # %bb.0: +; SSE2-NEXT: orps %xmm1, %xmm0 +; SSE2-NEXT: movmskps %xmm0, %eax +; SSE2-NEXT: testl %eax, %eax +; SSE2-NEXT: sete %al +; SSE2-NEXT: retq +; +; SSE41-LABEL: signtest_v8i32: +; SSE41: # %bb.0: +; SSE41-NEXT: por %xmm1, %xmm0 +; SSE41-NEXT: ptest {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0 +; SSE41-NEXT: sete %al +; SSE41-NEXT: retq +; +; AVX1-LABEL: signtest_v8i32: +; AVX1: # %bb.0: +; AVX1-NEXT: vptest {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %ymm0 +; AVX1-NEXT: sete %al +; AVX1-NEXT: vzeroupper +; AVX1-NEXT: retq +; +; AVX2-LABEL: signtest_v8i32: +; AVX2: # %bb.0: +; AVX2-NEXT: vpbroadcastq {{.*#+}} ymm1 = [9223372039002259456,9223372039002259456,9223372039002259456,9223372039002259456] +; AVX2-NEXT: vptest %ymm1, %ymm0 +; AVX2-NEXT: sete %al +; AVX2-NEXT: vzeroupper +; AVX2-NEXT: retq +; +; AVX512-LABEL: signtest_v8i32: +; AVX512: # %bb.0: +; AVX512-NEXT: vpbroadcastq {{.*#+}} ymm1 = [9223372039002259456,9223372039002259456,9223372039002259456,9223372039002259456] +; AVX512-NEXT: vptest %ymm1, %ymm0 +; AVX512-NEXT: sete %al +; AVX512-NEXT: vzeroupper +; AVX512-NEXT: retq + %1 = call i32 @llvm.vector.reduce.or.v8i32(<8 x i32> %a0) + %2 = icmp sgt i32 %1, -1 + ret i1 %2 +} + define i1 @trunc_v16i16(<16 x i16> %a0) { ; SSE2-LABEL: trunc_v16i16: ; SSE2: # %bb.0: @@ -1073,11 +1162,11 @@ define i32 @mask_v3i1(<3 x i32> %a, <3 x i32> %b) { ; SSE2-NEXT: movd %xmm0, %eax ; SSE2-NEXT: orl %ecx, %eax ; SSE2-NEXT: testb $1, %al -; SSE2-NEXT: je .LBB27_2 +; SSE2-NEXT: je .LBB29_2 ; SSE2-NEXT: # %bb.1: ; SSE2-NEXT: xorl %eax, %eax ; SSE2-NEXT: retq -; SSE2-NEXT: .LBB27_2: +; SSE2-NEXT: .LBB29_2: ; SSE2-NEXT: movl $1, %eax ; SSE2-NEXT: retq ; @@ -1092,11 +1181,11 @@ define i32 @mask_v3i1(<3 x i32> %a, <3 x i32> %b) { ; SSE41-NEXT: pextrd $2, %xmm1, %eax ; SSE41-NEXT: orl %ecx, %eax ; SSE41-NEXT: testb $1, %al -; SSE41-NEXT: je .LBB27_2 +; SSE41-NEXT: je .LBB29_2 ; SSE41-NEXT: # %bb.1: ; SSE41-NEXT: xorl %eax, %eax ; SSE41-NEXT: retq -; SSE41-NEXT: .LBB27_2: +; SSE41-NEXT: .LBB29_2: ; SSE41-NEXT: movl $1, %eax ; SSE41-NEXT: retq ; @@ -1111,11 +1200,11 @@ define i32 @mask_v3i1(<3 x i32> %a, <3 x i32> %b) { ; AVX1OR2-NEXT: vpextrd $2, %xmm0, %eax ; AVX1OR2-NEXT: orl %ecx, %eax ; AVX1OR2-NEXT: testb $1, %al -; AVX1OR2-NEXT: je .LBB27_2 +; AVX1OR2-NEXT: je .LBB29_2 ; AVX1OR2-NEXT: # %bb.1: ; AVX1OR2-NEXT: xorl %eax, %eax ; AVX1OR2-NEXT: retq -; AVX1OR2-NEXT: .LBB27_2: +; AVX1OR2-NEXT: .LBB29_2: ; AVX1OR2-NEXT: movl $1, %eax ; AVX1OR2-NEXT: retq ; @@ -1130,12 +1219,12 @@ define i32 @mask_v3i1(<3 x i32> %a, <3 x i32> %b) { ; AVX512F-NEXT: korw %k0, %k1, %k0 ; AVX512F-NEXT: kmovw %k0, %eax ; AVX512F-NEXT: testb $1, %al -; AVX512F-NEXT: je .LBB27_2 +; AVX512F-NEXT: je .LBB29_2 ; AVX512F-NEXT: # %bb.1: ; AVX512F-NEXT: xorl %eax, %eax ; AVX512F-NEXT: vzeroupper ; AVX512F-NEXT: retq -; AVX512F-NEXT: .LBB27_2: +; AVX512F-NEXT: .LBB29_2: ; AVX512F-NEXT: movl $1, %eax ; AVX512F-NEXT: vzeroupper ; AVX512F-NEXT: retq @@ -1151,12 +1240,12 @@ define i32 @mask_v3i1(<3 x i32> %a, <3 x i32> %b) { ; AVX512BW-NEXT: korw %k0, %k1, %k0 ; AVX512BW-NEXT: kmovd %k0, %eax ; AVX512BW-NEXT: testb $1, %al -; AVX512BW-NEXT: je .LBB27_2 +; AVX512BW-NEXT: je .LBB29_2 ; AVX512BW-NEXT: # %bb.1: ; AVX512BW-NEXT: xorl %eax, %eax ; AVX512BW-NEXT: vzeroupper ; AVX512BW-NEXT: retq -; AVX512BW-NEXT: .LBB27_2: +; AVX512BW-NEXT: .LBB29_2: ; AVX512BW-NEXT: movl $1, %eax ; AVX512BW-NEXT: vzeroupper ; AVX512BW-NEXT: retq @@ -1170,11 +1259,11 @@ define i32 @mask_v3i1(<3 x i32> %a, <3 x i32> %b) { ; AVX512BWVL-NEXT: korw %k0, %k1, %k0 ; AVX512BWVL-NEXT: kmovd %k0, %eax ; AVX512BWVL-NEXT: testb $1, %al -; AVX512BWVL-NEXT: je .LBB27_2 +; AVX512BWVL-NEXT: je .LBB29_2 ; AVX512BWVL-NEXT: # %bb.1: ; AVX512BWVL-NEXT: xorl %eax, %eax ; AVX512BWVL-NEXT: retq -; AVX512BWVL-NEXT: .LBB27_2: +; AVX512BWVL-NEXT: .LBB29_2: ; AVX512BWVL-NEXT: movl $1, %eax ; AVX512BWVL-NEXT: retq %1 = icmp ne <3 x i32> %a, %b From 85043c1c146fd5658ad4c5b5138e58994333e645 Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Wed, 20 Aug 2025 18:11:36 +0200 Subject: [PATCH 08/11] [Clang] Add a builtin that deduplicate types into a pack (#106730) The new builtin `__builtin_dedup_pack` removes duplicates from list of types. The added builtin is special in that they produce an unexpanded pack in the spirit of P3115R0 proposal. Produced packs can be used directly in template argument lists and get immediately expanded as soon as results of the computation are available. It allows to easily combine them, e.g.: ```cpp template struct Normalize { // Note: sort is not included in this PR, it illustrates the idea. using result = std::tuple< __builtin_sort_pack< __builtin_dedup_pack... >...>; } ; ``` Limitations: - only supported in template arguments and bases, - can only be used inside the templates, even if non-dependent, - the builtins cannot be assigned to template template parameters. The actual implementation proceeds as follows: - When the compiler encounters a `__builtin_dedup_pack` or other type-producing builtin with dependent arguments, it creates a dependent `TemplateSpecializationType`. - During substitution, if the template arguments are non-dependent, we will produce: a new type `SubstBuiltinTemplatePackType`, which stores an argument pack that needs to be substituted. This type is similar to the existing `SubstTemplateParmPack` in that it carries the argument pack that needs to be expanded further. The relevant code is shared. - On top of that, Clang also wraps the resulting type into `TemplateSpecializationType`, but this time only as a sugar. - To actually expand those packs, we collect the produced `SubstBuiltinTemplatePackType` inside `CollectUnexpandedPacks`. Because we know the size of the produces packs only after the initial substitution, places that do the actual expansion will need to have a second run over the substituted type to finalize the expansions (in this patch we only support this for template arguments, see `ExpandTemplateArgument`). If the expansion are requested in the places we do not currently support, we will produce an error. More follow-up work will be needed to fully shape this: - adding the builtin that sorts types, - remove the restrictions for expansions, - implementing P3115R0 (scheduled for C++29, see https://github.com/cplusplus/papers/issues/2300). --- .../clangd/unittests/FindTargetTests.cpp | 6 + clang/docs/LanguageExtensions.rst | 31 ++ clang/docs/ReleaseNotes.rst | 16 + clang/include/clang/AST/ASTContext.h | 3 + clang/include/clang/AST/DeclTemplate.h | 3 + clang/include/clang/AST/RecursiveASTVisitor.h | 30 +- clang/include/clang/AST/Type.h | 80 ++++- clang/include/clang/AST/TypeLoc.h | 22 +- clang/include/clang/AST/TypeProperties.td | 19 +- clang/include/clang/Basic/BuiltinTemplates.td | 4 + .../clang/Basic/DiagnosticSemaKinds.td | 7 + clang/include/clang/Basic/TypeNodes.td | 4 +- clang/include/clang/Sema/Sema.h | 18 +- clang/include/clang/Sema/SemaInternal.h | 11 +- .../clang/Serialization/TypeBitCodes.def | 1 + clang/lib/AST/ASTContext.cpp | 31 +- clang/lib/AST/ASTImporter.cpp | 8 + clang/lib/AST/ASTStructuralEquivalence.cpp | 8 + clang/lib/AST/DeclTemplate.cpp | 13 +- clang/lib/AST/ItaniumMangle.cpp | 15 + clang/lib/AST/MicrosoftMangle.cpp | 5 + clang/lib/AST/Type.cpp | 87 +++-- clang/lib/AST/TypePrinter.cpp | 10 + clang/lib/Parse/ParseTemplate.cpp | 2 + clang/lib/Sema/SemaConcept.cpp | 13 +- clang/lib/Sema/SemaDeclCXX.cpp | 3 +- clang/lib/Sema/SemaTemplate.cpp | 57 ++++ clang/lib/Sema/SemaTemplateDeduction.cpp | 18 +- clang/lib/Sema/SemaTemplateInstantiate.cpp | 177 ++++++++-- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 52 ++- clang/lib/Sema/SemaTemplateVariadic.cpp | 121 ++++++- clang/lib/Sema/TreeTransform.h | 319 ++++++++++++------ clang/lib/Serialization/ASTReader.cpp | 5 + clang/lib/Serialization/ASTWriter.cpp | 6 + .../test/Import/builtin-template/Inputs/S.cpp | 10 + clang/test/Import/builtin-template/test.cpp | 12 +- clang/test/PCH/dedup_types.cpp | 20 ++ clang/test/SemaCXX/pr100095.cpp | 1 - .../test/SemaTemplate/dedup-types-builtin.cpp | 226 +++++++++++++ clang/tools/libclang/CIndex.cpp | 1 + 40 files changed, 1238 insertions(+), 237 deletions(-) create mode 100644 clang/test/PCH/dedup_types.cpp create mode 100644 clang/test/SemaTemplate/dedup-types-builtin.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 20fd23ed4fcd..f369e1b0341e 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -731,6 +731,12 @@ TEST_F(TargetDeclTest, BuiltinTemplates) { using type_pack_element = [[__type_pack_element]]; )cpp"; EXPECT_DECLS("TemplateSpecializationTypeLoc", ); + + Code = R"cpp( + template