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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to