[clang] 96509bb - [Matrix] Preserve signedness when extending matrix index expression. (#103044)

[via cfe-commits]

Author: Florian Hahn
Date: 2024-08-23T10:11:52+01:00
New Revision: 96509bb98fc0a7e929304a64362baaa2589d5a6b

URL: https://github.com/llvm/llvm-project/commit/96509bb98fc0a7e929304a64362baaa2589d5a6b
DIFF: https://github.com/llvm/llvm-project/commit/96509bb98fc0a7e929304a64362baaa2589d5a6b.diff

LOG: [Matrix] Preserve signedness when extending matrix index expression. (#103044)

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

PR: https://github.com/llvm/llvm-project/pull/103044

Added: 
    

Modified: 
    clang/lib/CodeGen/CGExpr.cpp
    clang/lib/CodeGen/CGExprScalar.cpp
    clang/lib/CodeGen/CodeGenFunction.h
    clang/lib/Sema/SemaExpr.cpp
    clang/test/SemaCXX/matrix-index-operator-sign-conversion.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 48d9a3b8a5acb3..426fccb721db84 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4392,13 +4392,24 @@ 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());
-  llvm::Value *RowIdx = EmitScalarExpr(E->getRowIdx());
-  llvm::Value *ColIdx = EmitScalarExpr(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(),
       E->getBase()->getType()->castAs<ConstantMatrixType>()->getNumRows());

diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 3bda254c86adf6..2a726bba2dd304 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2007,8 +2007,8 @@ 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());
+  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 e1b9ada3c1e1fd..05f85f8b95bfa2 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);

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c67183df335dd5..ea57316ad8014e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5033,8 +5033,7 @@ ExprResult Sema::CreateBuiltinMatrixSubscriptExpr(Expr *Base, Expr *RowIdx,
       }
     }
 
-    ExprResult ConvExpr =
-        tryConvertExprToType(IndexExpr, Context.getSizeType());
+    ExprResult ConvExpr = IndexExpr;
     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 at -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 at -1 {{implicit conversion changes signedness: 'char' to 'unsigned long'}}
 
 double index4(m<double,3,1> X, int      i) { return X[0][i]; }
-// expected-warning at -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 at -1 {{implicit conversion changes signedness: 'char' to 'unsigned long'}}
+
+// expected-no-diagnostics