Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/139...@github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/139709 >From d90661aac34ae311f2d550ca0391e9e5232eb585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Tue, 13 May 2025 11:59:56 +0200 Subject: [PATCH 1/2] [clang] Save ShuffleVectorExpr args as ConstantExpr The passed indices have to be constant integers anyway, which we verify before creating the ShuffleVectorExpr. Use the value we create there and save the indices using a ConstantExpr instead. This way, we don't have to evaluate the args every time we call getShuffleMaskIdx(). --- clang/include/clang/AST/Expr.h | 6 ++++-- clang/lib/AST/ByteCode/Compiler.cpp | 2 +- clang/lib/AST/ExprConstant.cpp | 2 +- clang/lib/CodeGen/CGExprScalar.cpp | 2 +- clang/lib/Sema/SemaChecking.cpp | 22 +++++++++++----------- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index a83320a7ddec2..1e6749dda71fe 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -4566,9 +4566,11 @@ class ShuffleVectorExpr : public Expr { void setExprs(const ASTContext &C, ArrayRef<Expr *> Exprs); - llvm::APSInt getShuffleMaskIdx(const ASTContext &Ctx, unsigned N) const { + llvm::APSInt getShuffleMaskIdx(unsigned N) const { assert((N < NumExprs - 2) && "Shuffle idx out of range!"); - return getExpr(N+2)->EvaluateKnownConstInt(Ctx); + assert(isa<ConstantExpr>(getExpr(N + 2)) && + "Index expression must be a ConstantExpr"); + return cast<ConstantExpr>(getExpr(N + 2))->getAPValueResult().getInt(); } // Iterators diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index c7fb5e8466686..2702fc87b8235 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -3883,7 +3883,7 @@ bool Compiler<Emitter>::VisitShuffleVectorExpr(const ShuffleVectorExpr *E) { return false; } for (unsigned I = 0; I != NumOutputElems; ++I) { - APSInt ShuffleIndex = E->getShuffleMaskIdx(Ctx.getASTContext(), I); + APSInt ShuffleIndex = E->getShuffleMaskIdx(I); assert(ShuffleIndex >= -1); if (ShuffleIndex == -1) return this->emitInvalidShuffleVectorIndex(I, E); diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 500d43accb082..251508eb5f4c0 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -11558,7 +11558,7 @@ static bool handleVectorShuffle(EvalInfo &Info, const ShuffleVectorExpr *E, unsigned const TotalElementsInInputVector1 = VecVal1.getVectorLength(); unsigned const TotalElementsInInputVector2 = VecVal2.getVectorLength(); - APSInt IndexVal = E->getShuffleMaskIdx(Info.Ctx, EltNum); + APSInt IndexVal = E->getShuffleMaskIdx(EltNum); int64_t index = IndexVal.getExtValue(); // The spec says that -1 should be treated as undef for optimizations, // but in constexpr we'd have to produce an APValue::Indeterminate, diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 6765008c99c4a..7fe3a1660326b 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -1906,7 +1906,7 @@ Value *ScalarExprEmitter::VisitShuffleVectorExpr(ShuffleVectorExpr *E) { SmallVector<int, 32> Indices; for (unsigned i = 2; i < E->getNumSubExprs(); ++i) { - llvm::APSInt Idx = E->getShuffleMaskIdx(CGF.getContext(), i-2); + llvm::APSInt Idx = E->getShuffleMaskIdx(i - 2); // Check for -1 and output it as undef in the IR. if (Idx.isSigned() && Idx.isAllOnes()) Indices.push_back(-1); diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 55121b90fa167..d7c62b44a5c50 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -5342,29 +5342,29 @@ ExprResult Sema::BuiltinShuffleVector(CallExpr *TheCall) { } for (unsigned i = 2; i < TheCall->getNumArgs(); i++) { - if (TheCall->getArg(i)->isTypeDependent() || - TheCall->getArg(i)->isValueDependent()) + Expr *Arg = TheCall->getArg(i); + if (Arg->isTypeDependent() || Arg->isValueDependent()) continue; std::optional<llvm::APSInt> Result; - if (!(Result = TheCall->getArg(i)->getIntegerConstantExpr(Context))) + if (!(Result = Arg->getIntegerConstantExpr(Context))) return ExprError(Diag(TheCall->getBeginLoc(), diag::err_shufflevector_nonconstant_argument) - << TheCall->getArg(i)->getSourceRange()); + << Arg->getSourceRange()); // Allow -1 which will be translated to undef in the IR. if (Result->isSigned() && Result->isAllOnes()) - continue; - - if (Result->getActiveBits() > 64 || - Result->getZExtValue() >= numElements * 2) + ; + else if (Result->getActiveBits() > 64 || + Result->getZExtValue() >= numElements * 2) return ExprError(Diag(TheCall->getBeginLoc(), diag::err_shufflevector_argument_too_large) - << TheCall->getArg(i)->getSourceRange()); - } + << Arg->getSourceRange()); - SmallVector<Expr*, 32> exprs; + TheCall->setArg(i, ConstantExpr::Create(Context, Arg, APValue(*Result))); + } + SmallVector<Expr *> exprs; for (unsigned i = 0, e = TheCall->getNumArgs(); i != e; i++) { exprs.push_back(TheCall->getArg(i)); TheCall->setArg(i, nullptr); >From a07aac0f1337784689ba7aaacab52261f39727de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Wed, 14 May 2025 07:32:08 +0200 Subject: [PATCH 2/2] Does this fix the unit tests? --- clang/unittests/AST/ASTImporterTest.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index cddd301e22e50..2e7c90a2651d7 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -305,11 +305,13 @@ TEST_P(ImportExpr, ImportShuffleVectorExpr) { return __builtin_shufflevector(a, b, 0, 1, 2, 3); } )code"; - const auto Pattern = functionDecl(hasDescendant(shuffleVectorExpr( - allOf(has(declRefExpr(to(parmVarDecl(hasName("a"))))), - has(declRefExpr(to(parmVarDecl(hasName("b"))))), - has(integerLiteral(equals(0))), has(integerLiteral(equals(1))), - has(integerLiteral(equals(2))), has(integerLiteral(equals(3))))))); + const auto Pattern = functionDecl(hasDescendant( + shuffleVectorExpr(allOf(has(declRefExpr(to(parmVarDecl(hasName("a"))))), + has(declRefExpr(to(parmVarDecl(hasName("b"))))), + has(constantExpr(hasSubExpr(integerLiteral(equals(0))))), + has(constantExpr(hasSubExpr(integerLiteral(equals(1))))), + has(constantExpr(hasSubExpr(integerLiteral(equals(2))))), + has(constantExpr(hasSubExpr(integerLiteral(equals(3))))))))); testImport(Code, Lang_C99, "", Lang_C99, Verifier, Pattern); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
