[Matrix] Implement C-style explicit type conversions in CXX for matrix types

[SaurabhJha]

Summary

This patch implements C-style explicit type conversions in CXX for matrix types. It is part of fixing https://bugs.llvm.org/show_bug.cgi?id=47141

[fhahn]

Thanks for the patch!

It looks like there might be a few cases where the wrong conversion is used at the moment.

clang/test/CodeGenCXX/matrix-casts.cpp

27

Shouldn't this use zext for the conversion? Possibly that's an issue with the existing conversion code for matrixes?

67

Shouldn't this use sitofp for the conversion? Possibly that's an issue with the existing conversion code for matrixes?

clang/test/SemaCXX/matrix-casts.cpp

25–30

can you also add a test with an assignment without cast? This should be an error.

[SaurabhJha]

clang/test/CodeGenCXX/matrix-casts.cpp

27

Would definitely debug it but would like to quickly clarify this.

This is the code that's being executed. For int->int conversion, we are just checking the whether input is signed and regardless of whether output type is signed, we do CreateIntCast. We probably need to check OutputSigned too, right?

[fhahn]

clang/test/CodeGenCXX/matrix-casts.cpp

27

Nevermind, the scalar version I checked used zext due to some AArch64 specific optimization! So sext should be fine here. But that still leaves the int->float case. https://clang.godbolt.org/z/a5Tjed7sP

[SaurabhJha]

Updating D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

1. Remove bitcast when types are of the same size.
2. Make int conversion depend on whether both input and output are signed.
3. Add a test case of assigning to an incorrect type.

[SaurabhJha]

Thanks for your review @fhahn . I have made some changes in CGExprScalar.cpp to address your comments on IR. I have also added a statement to test assignment to incorrect type.

[SaurabhJha]

Remove unnecessary newline

[SaurabhJha]

clang/test/CodeGenCXX/matrix-casts.cpp

27

Whoops, did not see your updated comment. Reverting that change.

[SaurabhJha]

Revert change of sext -> zext

[SaurabhJha]

All comments addressed now :)

[fhahn]

Thanks for the update! Technically the fix in clang/lib/CodeGen/CGExprScalar.cpp is unrelated to C++ support. It would be great if you could put up a separate patch, so we can land this independently.

The whole patch basically LGTM. It would be great if you could add a test casting from unsigned to float/double (looks like no test generates uitofp) and a test for casting between signed & unsigned integers of the same size (unless there are already tests for that I missed)

clang/lib/CodeGen/CGExprScalar.cpp

1209

Could you s

clang/test/CodeGenCXX/matrix-casts.cpp

4

nit: drop the newline here and before using matrix_5_5...

[SaurabhJha]

In D101696#2732983, @fhahn wrote:

Thanks for the update! Technically the fix in clang/lib/CodeGen/CGExprScalar.cpp is unrelated to C++ support. It would be great if you could put up a separate patch, so we can land this independently.

The whole patch basically LGTM. It would be great if you could add a test casting from unsigned to float/double (looks like no test generates uitofp) and a test for casting between signed & unsigned integers of the same size (unless there are already tests for that I missed)

Sure thing. I have created a new patch here https://reviews.llvm.org/D101754. Once that's merged, I will rebase this one with the new main.

[SaurabhJha]

Rebase with main and add new tests for uitofp, unsigned int to int, and int to unsigned int

[fhahn]

LGTM, thanks!

I can land this on your behalf tomorrow.

After that I think you could apply for commit access https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access :)

[SaurabhJha]

In D101696#2734312, @fhahn wrote:

LGTM, thanks!

I can land this on your behalf tomorrow.

After that I think you could apply for commit access https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access :)

That sounds great, will do!

[SjoerdMeijer]

Perhaps for bonus points, update the Clang documentation in https://clang.llvm.org/docs/LanguageExtensions.html#matrix-types with some examples?

[SaurabhJha]

For sure, will do.

[SaurabhJha]

In D101696#2798713, @SjoerdMeijer wrote:

Perhaps for bonus points, update the Clang documentation in https://clang.llvm.org/docs/LanguageExtensions.html#matrix-types with some examples?

Can you please point me to how to submit patches to the documentation? I could only find links like https://clang.llvm.org/hacking.html which talk about code patches. Many thanks.

[SjoerdMeijer]

In D101696#2800790, @SaurabhJha wrote:

In D101696#2798713, @SjoerdMeijer wrote:

Perhaps for bonus points, update the Clang documentation in https://clang.llvm.org/docs/LanguageExtensions.html#matrix-types with some examples?

Can you please point me to how to submit patches to the documentation? I could only find links like https://clang.llvm.org/hacking.html which talk about code patches. Many thanks.

It works exactly the same as other patches. I think the file you're interested in is llvm-project/clang/docs/LanguageExtensions.rst.

clang/lib/CodeGen/CGExprScalar.cpp

Value *ScalarExprEmitter::EmitScalarCast(Value *Src, QualType SrcType,
                                          llvm::Type *DstTy,
                                          ScalarConversionOpts Opts) {
   // The Element types determine the type of cast to perform.
   llvm::Type *SrcElementTy;
   llvm::Type *DstElementTy;
   QualType SrcElementType;
   QualType DstElementType;
   if (SrcType->isMatrixType() && DstType->isMatrixType()) {
-    // Allow bitcast between matrixes of the same size.
-    if (SrcTy->getPrimitiveSizeInBits() == DstTy->getPrimitiveSizeInBits())
-      return Builder.CreateBitCast(Src, DstTy, "conv");
-
     SrcElementTy = cast<llvm::VectorType>(SrcTy)->getElementType();
     DstElementTy = cast<llvm::VectorType>(DstTy)->getElementType();
     SrcElementType = SrcType->castAs<MatrixType>()->getElementType();
     DstElementType = DstType->castAs<MatrixType>()->getElementType();
   } else {
     assert(!SrcType->isMatrixType() && !DstType->isMatrixType() &&
            "cannot cast between matrix and non-matrix types");
     SrcElementTy = SrcTy;

clang/lib/Sema/SemaCast.cpp

void CastOperation::CheckCXXCStyleCast(bool FunctionalStyle,
 
   if (ValueKind == VK_RValue && !DestType->isRecordType() &&
       !isPlaceholder(BuiltinType::Overload)) {
     SrcExpr = Self.DefaultFunctionArrayLvalueConversion(SrcExpr.get());
     if (SrcExpr.isInvalid())
       return;
   }
 
+  if (DestType->getAs<MatrixType>() ||
+      SrcExpr.get()->getType()->getAs<MatrixType>()) {
+    if (Self.CheckMatrixCast(OpRange, DestType, SrcExpr.get()->getType(), Kind))
+      SrcExpr = ExprError();
+    return;
+  }
+
   // AltiVec vector initialization with a single literal.
   if (const VectorType *vecTy = DestType->getAs<VectorType>())
     if (vecTy->getVectorKind() == VectorType::AltiVecVector
         && (SrcExpr.get()->getType()->isIntegerType()
             || SrcExpr.get()->getType()->isFloatingType())) {
       Kind = CK_VectorSplat;
       SrcExpr = Self.prepareVectorSplat(DestType, SrcExpr.get());
       return;

clang/test/CodeGen/matrix-cast.c

void cast_int_matrix_to_short(ix5x5 i, sx5x5 s) {
   // CHECK-NEXT:  ret void
 
   s = (sx5x5)i;
 }
 
 void cast_int_matrix_to_float(ix5x5 i, fx5x5 f) {
   // CHECK-LABEL: define{{.*}} void @cast_int_matrix_to_float(<25 x i32> %i, <25 x float> %f)
   // CHECK:       [[I:%.*]] = load <25 x i32>, <25 x i32>* {{.*}}, align 4
-  // CHECK-NEXT:  [[CONV:%.*]] = bitcast <25 x i32> [[I]] to <25 x float>
+  // CHECK-NEXT:  [[CONV:%.*]] = sitofp <25 x i32> [[I]] to <25 x float>
   // CHECK-NEXT:  store <25 x float> [[CONV]], <25 x float>* {{.*}}, align 4
   // CHECK-NEXT:  ret void
 
   f = (fx5x5)i;
 }
 
 void cast_double_matrix_to_int(dx5x5 d, ix5x5 i) {
   // CHECK-LABEL: define{{.*}} void @cast_double_matrix_to_int(<25 x double> %d, <25 x i32> %i)

clang/test/CodeGenCXX/matrix-casts.cpp

+// RUN: %clang_cc1 -std=c++11 -fenable-matrix -triple x86_64-apple-darwin %s -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+template <typename X>
+
+using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+template <typename Y>
+
+using matrix_5_5 = Y __attribute__((matrix_type(5, 5)));
+
+// CHECK-LABEL: define{{.*}} void @_Z19CastCharMatrixToIntv
+void CastCharMatrixToInt() {
+  // CHECK: [[C:%.*]] = load <25 x i8>, <25 x i8>* {{.*}}, align 1
+  // CHECK-NEXT: [[CONV:%.*]] = sext <25 x i8> [[C]] to <25 x i32>
+  // CHECK-NEXT: [[CONV1:%.*]] = bitcast [25 x i32]* {{.*}} to <25 x i32>*
+  // CHECK-NEXT: store <25 x i32> [[CONV]], <25 x i32>* [[CONV1]], align 4
+
+  matrix_5_5<char> c;
+  matrix_5_5<int> i;
+  i = (matrix_5_5<int>)c;
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z27CastCharMatrixToUnsignedIntv
+void CastCharMatrixToUnsignedInt() {
+  // CHECK:       [[C:%.*]] = load <25 x i8>, <25 x i8>* {{.*}}, align 1
+  // CHECK-NEXT:  [[CONV:%.*]] = sext <25 x i8> [[C]] to <25 x i32>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i32]* {{.*}} to <25 x i32>*
+  // CHECK-NEXT:  store <25 x i32> [[CONV]], <25 x i32>* [[CONV1]], align 4
+  // CHECK-NEXT:  ret void
+
+  matrix_5_5<char> c;
+  matrix_5_5<unsigned int> u;
+  u = (matrix_5_5<unsigned int>)c;
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z32CastUnsignedLongIntMatrixToShortv
+void CastUnsignedLongIntMatrixToShort() {
+  // CHECK:      [[U:%.*]] = load <25 x i64>, <25 x i64>* {{.*}}, align 8
+  // CHECK-NEXT: [[CONV:%.*]] = trunc <25 x i64> {{.*}} to <25 x i16>
+  // CHECK-NEXT: [[CONV1:%.*]] = bitcast [25 x i16]* {{.*}} to <25 x i16>*
+  // CHECK-NEXT: store <25 x i16> [[CONV]], <25 x i16>* [[CONV1]], align 2
+  // CHECK-NEXT: ret void
+
+  matrix_5_5<unsigned long int> u;
+  matrix_5_5<short int> s;
+  s = (matrix_5_5<short int>)u;
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z20CastIntMatrixToShortv()
+void CastIntMatrixToShort() {
+  // CHECK:       [[I:%.*]] = load <25 x i32>, <25 x i32>* {{.*}}, align 4
+  // CHECK-NEXT:  [[CONV:%.*]] = trunc <25 x i32> [[I]] to <25 x i16>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i16]* {{.*}} to <25 x i16>*
+  // CHECK-NEXT:  store <25 x i16> [[CONV]], <25 x i16>* [[CONV1]], align 2
+  // CHECK-NEXT:  ret void
+
+  matrix_5_5<int> i;
+  matrix_5_5<short int> s;
+  s = (matrix_5_5<short int>)i;
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z20CastIntMatrixToFloatv()
+void CastIntMatrixToFloat() {
+  // CHECK:       [[I:%.*]] = load <25 x i32>, <25 x i32>* {{.*}}, align 4
+  // CHECK-NEXT:  [[CONV]] = sitofp <25 x i32> {{.*}} to <25 x float>
+  // CHECK-NEXT:  [[CONV1]] = bitcast [25 x float]* {{.*}} to <25 x float>*
+  // CHECK-NEXT:  store <25 x float> [[CONV]], <25 x float>* [[CONV1]], align 4
+  // CHECK-NEXT:  ret void
+
+  matrix_5_5<int> i;
+  matrix_5_5<float> f;
+  f = (matrix_5_5<float>)i;
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z21CastDoubleMatrixToIntv()
+void CastDoubleMatrixToInt() {
+  // CHECK:       [[D:%.*]] = load <25 x double>, <25 x double>* {{.*}}, align 8
+  // CHECK-NEXT:  [[CONV:%.*]] = fptosi <25 x double> [[D]] to <25 x i32>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i32]* %i to <25 x i32>*
+  // CHECK-NEXT:  store <25 x i32> [[CONV]], <25 x i32>* {{.*}}, align 4
+  // CHECK-NEXT:  ret void
+
+  matrix_5_5<double> d;
+  matrix_5_5<int> i;
+  i = (matrix_5_5<int>)d;
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z33CastFloatMatrixToUnsignedShortIntv()
+void CastFloatMatrixToUnsignedShortInt() {
+  // CHECK:       [[F:%.*]] = load <25 x float>, <25 x float>* {{.*}}, align 4
+  // CHECK-NEXT:  [[CONV:%.*]] = fptoui <25 x float> [[F]] to <25 x i16>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i16]* {{.*}} to <25 x i16>*
+  // CHECK-NEXT:  store <25 x i16> [[CONV]], <25 x i16>* [[CONV1]], align 2
+  // CHECK-NEXT:  ret void
+
+  matrix_5_5<float> f;
+  matrix_5_5<unsigned short int> i;
+  i = (matrix_5_5<unsigned short int>)f;
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z23CastDoubleMatrixToFloatv()
+void CastDoubleMatrixToFloat() {
+  // CHECK:       [[D:%.*]] = load <25 x double>, <25 x double>* {{.*}}, align 8
+  // CHECK-NEXT:  [[CONV:%.*]] = fptrunc <25 x double> [[D]] to <25 x float>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x float]* {{.*}} to <25 x float>*
+  // CHECK-NEXT:  store <25 x float> [[CONV]], <25 x float>* [[CONV1]], align 4
+  // CHECK-NEXT:  ret void
+
+  matrix_5_5<double> d;
+  matrix_5_5<float> f;
+  f = (matrix_5_5<float>)d;
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z33CastUnsignedShortIntToUnsignedIntv()
+void CastUnsignedShortIntToUnsignedInt() {
+  // CHECK:       [[S:%.*]] = load <25 x i16>, <25 x i16>* {{.*}}, align 2
+  // CHECK-NEXT:  [[CONV:%.*]] = zext <25 x i16> [[S]] to <25 x i32>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i32]* {{.*}} to <25 x i32>*
+  // CHECK-NEXT:  store <25 x i32> [[CONV]], <25 x i32>* [[CONV1]], align 4
+  // CHECK-NEXT:  ret void
+
+  matrix_5_5<unsigned short int> s;
+  matrix_5_5<unsigned int> i;
+  i = (matrix_5_5<unsigned int>)s;
+}
+
+void CastUnsignedLongIntToUnsignedShortInt() {
+  // CHECK:       [[L:%.*]] = load <25 x i64>, <25 x i64>* %0, align 8
+  // CHECK-NEXT:  [[CONV:%.*]] = trunc <25 x i64> [[L]] to <25 x i16>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i16]* {{.*}} to <25 x i16>*
+  // CHECK-NEXT:  store <25 x i16> [[CONV]], <25 x i16>* [[CONV1]], align 2
+  // CHECK-NEXT:  ret void
+
+  matrix_5_5<unsigned long int> l;
+  matrix_5_5<unsigned short int> s;
+  s = (matrix_5_5<unsigned short int>)l;
+}
 No newline at end of file

clang/test/SemaCXX/matrix-casts.cpp

 // RUN: %clang_cc1 -std=c++11 -fenable-matrix -fsyntax-only -verify %s
 
 template <typename X>
 
 using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
 
 template <typename Y>
 
 using matrix_5_5 = Y __attribute__((matrix_type(5, 5)));
 
 typedef struct test_struct {
 } test_struct;
 
 typedef int vec __attribute__((vector_size(4)));
 
 void f1() {
-  // TODO: Update this test once the support of C-style casts for C++ is implemented.
   matrix_4_4<char> m1;
   matrix_4_4<int> m2;
   matrix_4_4<short> m3;
   matrix_5_5<int> m4;
   int i;
   vec v;
   test_struct *s;
 
-  (matrix_4_4<int>)m1;   // expected-error {{C-style cast from 'matrix_4_4<char>' (aka 'char __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4<int>' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_4_4<short>)m2; // expected-error {{C-style cast from 'matrix_4_4<int>' (aka 'int __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4<short>' (aka 'short __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_5_5<int>)m3;   // expected-error {{C-style cast from 'matrix_4_4<short>' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'matrix_5_5<int>' (aka 'int __attribute__((matrix_type(5, 5)))') is not allowed}}
-
-  (int)m3;            // expected-error {{C-style cast from 'matrix_4_4<short>' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'int'}}
-  (matrix_4_4<int>)i; // expected-error {{C-style cast from 'int' to 'matrix_4_4<int>' (aka 'int __attribute__((\
-matrix_type(4, 4)))') is not allowed}}
-
-  (vec) m2;            // expected-error {{C-style cast from 'matrix_4_4<int>' (aka 'int __attribute__((matrix_type(4, 4)))') \
-to 'vec' (vector of 1 'int' value) is not allowed}}
-  (matrix_4_4<char>)v; // expected-error {{C-style cast from 'vec' (vector of 1 'int' value) to 'matrix_4_4<char>' \
-(aka 'char __attribute__((matrix_type(4, 4)))') is not allowed}}
-
-  (test_struct *)m1;    // expected-error {{cannot cast from type 'matrix_4_4<char>' (aka 'char __attribute__\
-((matrix_type(4, 4)))') to pointer type 'test_struct *}}'
-  (matrix_5_5<float>)s; // expected-error {{C-style cast from 'test_struct *' to 'matrix_5_5<float>' (aka 'float __attribute__\
-((matrix_type(5, 5)))') is not allowed}}'
+  m2 = (matrix_4_4<int>)m1;
+  m2 = m1; // expected-error {{assigning to 'matrix_4_4<int>' from incompatible type 'matrix_4_4<char>'}}
+  m3 = (matrix_4_4<short>)m2;
+  (matrix_5_5<int>)m3; // expected-error {{conversion between matrix types 'matrix_5_5<int>' (aka 'int __attribute__\
+((matrix_type(5, 5)))') and 'matrix_4_4<short>' (aka 'short __attribute__((matrix_type(4, 4)))') of different size is not\
+ allowed}}
+
+  (int)m3;            // expected-error {{conversion between matrix type 'matrix_4_4<short>' (aka 'short __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'int' is not allowed}}
+  (matrix_4_4<int>)i; // expected-error {{conversion between matrix type 'matrix_4_4<int>' (aka 'int __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'int' is not allowed}}
+
+  (vec) m2;            // expected-error {{conversion between matrix type 'matrix_4_4<int>' (aka 'int __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'vec' (vector of 1 'int' value) is not allowed}}
+  (matrix_4_4<char>)v; // expected-error {{conversion between matrix type 'matrix_4_4<char>' (aka 'char __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'vec' (vector of 1 'int' value) is not allowed}}
+
+  (test_struct *)m1;    // expected-error {{conversion between matrix type 'matrix_4_4<char>' (aka 'char __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'test_struct *' is not allowed}}'
+  (matrix_5_5<float>)s; // expected-error {{conversion between matrix type 'matrix_5_5<float>' (aka 'float __attribute__\
+((matrix_type(5, 5)))') and incompatible type 'test_struct *' is not allowed}}'
 }
 
 void f2() {
-  // TODO: Update this test once the support of C-style casts for C++ is implemented.
   matrix_4_4<float> m1;
-  matrix_5_5<double> m2;
-  matrix_5_5<signed int> m3;
-  matrix_4_4<unsigned int> m4;
+  matrix_4_4<double> m2;
+  matrix_5_5<double> m3;
+  matrix_5_5<signed int> m4;
+  matrix_4_4<unsigned int> m5;
+  matrix_5_5<unsigned int> m6;
   float f;
 
-  (matrix_4_4<double>)m1;       // expected-error {{C-style cast from 'matrix_4_4<float>' (aka 'float __attribute__\
-((matrix_type(4, 4)))') to 'matrix_4_4<double>' (aka 'double __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_5_5<float>)m2;        // expected-error {{C-style cast from 'matrix_5_5<double>' (aka 'double __attribute__\
-((matrix_type(5, 5)))') to 'matrix_5_5<float>' (aka 'float __attribute__((matrix_type(5, 5)))') is not allowed}}
-  (matrix_5_5<unsigned int>)m3; // expected-error {{C-style cast from 'matrix_5_5<int>' (aka 'int __attribute__\
-((matrix_type(5, 5)))') to 'matrix_5_5<unsigned int>' (aka 'unsigned int __attribute__((matrix_type(5, 5)))') \
-is not allowed}}
-  (matrix_4_4<int>)m4;          // expected-error {{C-style cast from 'matrix_4_4<unsigned int>' (aka 'unsigned int \
-__attribute__((matrix_type(4, 4)))') to 'matrix_4_4<int>' (aka 'int __attribute__((matrix_type(4, 4)))') is not \
-allowed}}
+  m2 = (matrix_4_4<double>)m1;
+  (matrix_5_5<double>)m1; // expected-error {{conversion between matrix types 'matrix_5_5<double>' (aka 'double __\
+attribute__((matrix_type(5, 5)))') and 'matrix_4_4<float>' (aka 'float __attribute__((matrix_type(4, 4)))') of different\
+ size is not allowed}}
+  m4 = (matrix_5_5<signed int>)m3;
+  m5 = (matrix_5_5<unsigned int>)m4; // expected-error {{assigning to 'matrix_4_4<unsigned int>' (aka 'unsigned int \
+__attribute__((matrix_type(4, 4)))') from incompatible type 'matrix_5_5<unsigned int>' (aka 'unsigned int __attribute__\
+((matrix_type(5, 5)))')}}
+  m6 = (matrix_5_5<unsigned int>)m4;
+  m4 = (matrix_5_5<signed int>)m6;
 }