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

[Florian Hahn via Phabricator via cfe-commits]

fhahn requested changes to this revision.
fhahn added a comment.
This revision now requires changes to proceed.

Thanks for the patch!

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



================
Comment at: clang/test/CodeGenCXX/matrix-casts.cpp:26
+  // 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>*
----------------
Shouldn't this use `zext` for the conversion? Possibly that's an issue with the existing conversion code for matrixes?


================
Comment at: clang/test/CodeGenCXX/matrix-casts.cpp:66
+  // CHECK-NEXT:  [[CONV]] = bitcast <25 x i32> %1 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
----------------
Shouldn't this use `sitofp` for the conversion? Possibly that's an issue with the existing conversion code for matrixes?


================
Comment at: clang/test/SemaCXX/matrix-casts.cpp:25
 
-  (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}}
+  m2 = (matrix_4_4<int>)m1;
+  m3 = (matrix_4_4<short>)m2;
----------------
can you also add a test with an assignment without cast? This should be an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696