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 fa8d8e3f1c786f096b8fc23e38b398e63363406d 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 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index cddd301e22e50..190bf64d35eda 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -308,8 +308,10 @@ TEST_P(ImportExpr, ImportShuffleVectorExpr) { 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))))))); + has(constantExpr(has(integerLiteral(equals(0))))), + has(constantExpr(has(integerLiteral(equals(1))))), + has(constantExpr(has(integerLiteral(equals(2))))), + has(constantExpr(has(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
