https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/103044
>From 3fc96327079c04e9bcac9488d0ee03e61bb5a3fb Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Tue, 13 Aug 2024 12:28:34 +0100 Subject: [PATCH 1/3] [Matrix] Preserve signedness when extending matrix index expression. As per [1] the indices for a matrix element access operator shall have integral or unscoped enumeration types and be non-negative. At the moment, the index expression is converted to SizeType irrespective of the signedness of the index expression. This causes implicit sign conversion warnings if any of the indices is signed. As per the spec, using signed types as indices is allowed and should not cause any warnings. If the index expression is signed, extend to SignedSizeType to avoid the warning. [1] https://clang.llvm.org/docs/MatrixTypes.html#matrix-type-element-access-operator --- clang/lib/Sema/SemaExpr.cpp | 6 ++++-- .../test/SemaCXX/matrix-index-operator-sign-conversion.cpp | 7 ++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 0f58eb2840211d..258e931ef592e5 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5025,8 +5025,10 @@ ExprResult Sema::CreateBuiltinMatrixSubscriptExpr(Expr *Base, Expr *RowIdx, } } - ExprResult ConvExpr = - tryConvertExprToType(IndexExpr, Context.getSizeType()); + ExprResult ConvExpr = tryConvertExprToType( + IndexExpr, IndexExpr->getType()->isSignedIntegerType() + ? Context.getSignedSizeType() + : Context.getSizeType()); assert(!ConvExpr.isInvalid() && "should be able to convert any integer type to size type"); return ConvExpr.get(); diff --git a/clang/test/SemaCXX/matrix-index-operator-sign-conversion.cpp b/clang/test/SemaCXX/matrix-index-operator-sign-conversion.cpp index 4254780651c5f5..e6fe4a6c57ff22 100644 --- a/clang/test/SemaCXX/matrix-index-operator-sign-conversion.cpp +++ b/clang/test/SemaCXX/matrix-index-operator-sign-conversion.cpp @@ -2,19 +2,16 @@ template <typename T, int R, int C> using m __attribute__((__matrix_type__(R,C))) = T; -// FIXME: should not warn here. double index1(m<double,3,1> X, int i) { return X[i][0]; } -// expected-warning@-1 {{implicit conversion changes signedness: 'int' to 'unsigned long'}} double index2(m<double,3,1> X, unsigned i) { return X[i][0]; } double index3(m<double,3,1> X, char i) { return X[i][0]; } -// expected-warning@-1 {{implicit conversion changes signedness: 'char' to 'unsigned long'}} double index4(m<double,3,1> X, int i) { return X[0][i]; } -// expected-warning@-1 {{implicit conversion changes signedness: 'int' to 'unsigned long'}} double index5(m<double,3,1> X, unsigned i) { return X[0][i]; } double index6(m<double,3,1> X, char i) { return X[0][i]; } -// expected-warning@-1 {{implicit conversion changes signedness: 'char' to 'unsigned long'}} + +// expected-no-diagnostics >From 4efcce858df40edb0fbe70b296bfb0b4e0cd78d3 Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Thu, 15 Aug 2024 17:29:42 +0100 Subject: [PATCH 2/3] !fixup move extends to CG. --- clang/lib/CodeGen/CGExpr.cpp | 14 ++++++++++++-- clang/lib/CodeGen/CGExprScalar.cpp | 11 +++++++++-- clang/lib/Sema/SemaExpr.cpp | 5 +---- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 48d9a3b8a5acb3..866c6f5be36c42 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4397,8 +4397,18 @@ LValue CodeGenFunction::EmitMatrixSubscriptExpr(const MatrixSubscriptExpr *E) { !E->isIncomplete() && "incomplete matrix subscript expressions should be rejected during Sema"); LValue Base = EmitLValue(E->getBase()); - llvm::Value *RowIdx = EmitScalarExpr(E->getRowIdx()); - llvm::Value *ColIdx = EmitScalarExpr(E->getColumnIdx()); + + // Extend or truncate the index type to 32 or 64-bits. + auto EmitIndex = [this](const Expr *E) { + llvm::Value *Idx = EmitScalarExpr(E); + bool IsSigned = E->getType()->isSignedIntegerOrEnumerationType(); + if (Idx->getType() != IntPtrTy) + Idx = Builder.CreateIntCast(Idx, IntPtrTy, IsSigned); + return Idx; + }; + llvm::Value *RowIdx = EmitIndex(E->getRowIdx()); + llvm::Value *ColIdx = EmitIndex(E->getColumnIdx()); + llvm::Value *NumRows = Builder.getIntN( RowIdx->getType()->getScalarSizeInBits(), E->getBase()->getType()->castAs<ConstantMatrixType>()->getNumRows()); diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 84392745ea6144..200d385103d593 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -1995,8 +1995,15 @@ Value *ScalarExprEmitter::VisitMatrixSubscriptExpr(MatrixSubscriptExpr *E) { // Handle the vector case. The base must be a vector, the index must be an // integer value. - Value *RowIdx = Visit(E->getRowIdx()); - Value *ColumnIdx = Visit(E->getColumnIdx()); + auto VisitIndex = [this](Expr *E) { + llvm::Value *Idx = Visit(E); + bool IsSigned = E->getType()->isSignedIntegerOrEnumerationType(); + if (Idx->getType() != CGF.IntPtrTy) + Idx = Builder.CreateIntCast(Idx, CGF.IntPtrTy, IsSigned); + return Idx; + }; + Value *RowIdx = VisitIndex(E->getRowIdx()); + Value *ColumnIdx = VisitIndex(E->getColumnIdx()); const auto *MatrixTy = E->getBase()->getType()->castAs<ConstantMatrixType>(); unsigned NumRows = MatrixTy->getNumRows(); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 258e931ef592e5..80442cb25d0ab6 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5025,10 +5025,7 @@ ExprResult Sema::CreateBuiltinMatrixSubscriptExpr(Expr *Base, Expr *RowIdx, } } - ExprResult ConvExpr = tryConvertExprToType( - IndexExpr, IndexExpr->getType()->isSignedIntegerType() - ? Context.getSignedSizeType() - : Context.getSizeType()); + ExprResult ConvExpr = IndexExpr; assert(!ConvExpr.isInvalid() && "should be able to convert any integer type to size type"); return ConvExpr.get(); >From e05725ebe123ea2888c65073ec272e13bbcfa83e Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Fri, 16 Aug 2024 09:43:15 +0100 Subject: [PATCH 3/3] !fixup move to CGF::EmitMatrixIndexExpr --- clang/lib/CodeGen/CGExpr.cpp | 21 +++++++++++---------- clang/lib/CodeGen/CGExprScalar.cpp | 11 ++--------- clang/lib/CodeGen/CodeGenFunction.h | 1 + 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 866c6f5be36c42..426fccb721db84 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4392,22 +4392,23 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E, return LV; } +llvm::Value *CodeGenFunction::EmitMatrixIndexExpr(const Expr *E) { + llvm::Value *Idx = EmitScalarExpr(E); + if (Idx->getType() == IntPtrTy) + return Idx; + bool IsSigned = E->getType()->isSignedIntegerOrEnumerationType(); + return Builder.CreateIntCast(Idx, IntPtrTy, IsSigned); +} + LValue CodeGenFunction::EmitMatrixSubscriptExpr(const MatrixSubscriptExpr *E) { assert( !E->isIncomplete() && "incomplete matrix subscript expressions should be rejected during Sema"); LValue Base = EmitLValue(E->getBase()); - // Extend or truncate the index type to 32 or 64-bits. - auto EmitIndex = [this](const Expr *E) { - llvm::Value *Idx = EmitScalarExpr(E); - bool IsSigned = E->getType()->isSignedIntegerOrEnumerationType(); - if (Idx->getType() != IntPtrTy) - Idx = Builder.CreateIntCast(Idx, IntPtrTy, IsSigned); - return Idx; - }; - llvm::Value *RowIdx = EmitIndex(E->getRowIdx()); - llvm::Value *ColIdx = EmitIndex(E->getColumnIdx()); + // Extend or truncate the index type to 32 or 64-bits if needed. + llvm::Value *RowIdx = EmitMatrixIndexExpr(E->getRowIdx()); + llvm::Value *ColIdx = EmitMatrixIndexExpr(E->getColumnIdx()); llvm::Value *NumRows = Builder.getIntN( RowIdx->getType()->getScalarSizeInBits(), diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 200d385103d593..e1f14fd36ee98f 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -1995,15 +1995,8 @@ Value *ScalarExprEmitter::VisitMatrixSubscriptExpr(MatrixSubscriptExpr *E) { // Handle the vector case. The base must be a vector, the index must be an // integer value. - auto VisitIndex = [this](Expr *E) { - llvm::Value *Idx = Visit(E); - bool IsSigned = E->getType()->isSignedIntegerOrEnumerationType(); - if (Idx->getType() != CGF.IntPtrTy) - Idx = Builder.CreateIntCast(Idx, CGF.IntPtrTy, IsSigned); - return Idx; - }; - Value *RowIdx = VisitIndex(E->getRowIdx()); - Value *ColumnIdx = VisitIndex(E->getColumnIdx()); + Value *RowIdx = CGF.EmitMatrixIndexExpr(E->getRowIdx()); + Value *ColumnIdx = CGF.EmitMatrixIndexExpr(E->getColumnIdx()); const auto *MatrixTy = E->getBase()->getType()->castAs<ConstantMatrixType>(); unsigned NumRows = MatrixTy->getNumRows(); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 57e0b7f91e9bf8..5593e9f791b963 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -4280,6 +4280,7 @@ class CodeGenFunction : public CodeGenTypeCache { LValue EmitUnaryOpLValue(const UnaryOperator *E); LValue EmitArraySubscriptExpr(const ArraySubscriptExpr *E, bool Accessed = false); + llvm::Value *EmitMatrixIndexExpr(const Expr *E); LValue EmitMatrixSubscriptExpr(const MatrixSubscriptExpr *E); LValue EmitArraySectionExpr(const ArraySectionExpr *E, bool IsLowerBound = true); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
