[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

[Florian Hahn via Phabricator via cfe-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for working on this!



================
Comment at: clang/include/clang/Sema/Sema.h:11715
+  // CheckMatrixCast - Check type constraints for matrix casts.
+  // We allow casting between matrices of the same dimensions i.e. when they
+  // have the same number of rows and column. Returns true if the cast is
----------------
Nit: matrixes for consistency


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1201
                                          ScalarConversionOpts Opts) {
-  if (isa<llvm::IntegerType>(SrcTy)) {
-    bool InputSigned = SrcType->isSignedIntegerOrEnumerationType();
-    if (SrcType->isBooleanType() && Opts.TreatBooleanAsSigned) {
+  // If the types are non-matrix, the element types and the types are the same.
+  // The assignments in this condition are used in the code following this
----------------
I think the comment here could be a bit shorter and to the point. Perhaps something like `The *Element* types are used to determine the kind of cast to perform.`

The assignments are just below and probably are clear from the code.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1210
+  if (SrcType->isMatrixType() && DstType->isMatrixType()) {
+    // Allow bitcast between matrices of the same size.
+    if (SrcTy->getPrimitiveSizeInBits() == DstTy->getPrimitiveSizeInBits())
----------------
nit: matrixes for consistency


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99037/new/

https://reviews.llvm.org/D99037