fhahn added a comment.
Thanks for the latest update! This basically looks good to me, with a few final
nits!
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8584
+def err_invalid_conversion_between_matrixes : Error<
+ "conversion between matrix type%diff{ $ and $|}0,1 of different size is not
allowed">;
+
----------------
nit: `matrix types`?
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8586
+
+def err_invalid_conversion_between_matrix_and_type : Error<
+ "conversion between matrix type %0 and type %1 is not allowed">;
----------------
nit: `_and_incompatible_type`?
================
Comment at: clang/include/clang/Sema/Sema.h:11718
+ // invalid.
+ bool CheckMatrixCast(SourceRange R, QualType MatrixTy, QualType Tr,
+ CastKind &Kind);
----------------
Can the name for `Tr` be improved? Perhaps `ToTy` or `TargetTy`?
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1219
+ } else {
+ assert(!SrcType->isMatrixType() && !DstType->isMatrixType());
+ SrcElementTy = SrcTy;
----------------
Thanks! Can you also add a message to the assert , like `&& "cannot cast
between matrix and non-matrix types")`
================
Comment at: clang/lib/Sema/SemaCast.cpp:2932
+ if (DestType->getAs<MatrixType>() || SrcType->getAs<MatrixType>()) {
+ if (Self.CheckMatrixCast(OpRange, DestType, SrcType, Kind)) {
+ SrcExpr = ExprError();
----------------
nit: no braces.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:7357
+
+ return (matSrcType->getNumRows() == matDestType->getNumRows() &&
+ matSrcType->getNumColumns() == matDestType->getNumColumns());
----------------
nit: No `( ...... )` required here.
================
Comment at: clang/test/CodeGen/matrix-cast.c:82
+
+void cast_unsigned_short_int_to_unsigned_int(unsigned_short_int_5x5 s,
unsigned_int_5x5 i) {
+ // CHECK-LABEL: define{{.*}} void
@cast_unsigned_short_int_to_unsigned_int(<25 x i16> %s, <25 x i32> %i)
----------------
I think this is still missing a test from unsigned to signed?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99037/new/
https://reviews.llvm.org/D99037
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
