Skip to content

Commit

Permalink
[Matrix] Preserve signedness when extending matrix index expression. (l…
Browse files Browse the repository at this point in the history
…lvm#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: llvm#103044
  • Loading branch information
fhahn authored and dmpolukhin committed Sep 2, 2024
1 parent 0e40538 commit f99b0ec
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 11 deletions.
15 changes: 13 additions & 2 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CGExprScalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions clang/lib/CodeGen/CodeGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 2 additions & 5 deletions clang/test/SemaCXX/matrix-index-operator-sign-conversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit f99b0ec

Please sign in to comment.