fhahn added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:547
+ case CK_MatrixCast: {
+ // TODO: Handle MatrixCast here.
+ }
----------------
SaurabhJha wrote:
> I thought doing changes here is is outside the scope of casting so I just
> left a TODO here. Please let me know if we want to do something else here.
I think that's fair, but you should probably add `continue;` rather than
falling through.
================
Comment at: clang/test/CodeGen/matrix-cast.c:15
+ // CHECK: [[C:%.*]] = load <25 x i8>, <25 x i8>* {{.*}}, align 1
+ // CHECK-NEXT: [[CONV:%.*]] = zext <25 x i8> [[C]] to <25 x i32>
+ // CHECK-NEXT: store <25 x i32> [[CONV]], <25 x i32>* {{.*}}, align 4
----------------
This doesn't seem right. We are casting between 2 signed types, so the sign
should get preserved, right? Shouldn't this be `sext`? See
https://godbolt.org/z/zWznYdnKW for the scalar case.
I think you also need tests for casts with different bitwidths with unsigned,
unsigned -> signed & signed -> unsigned.
================
Comment at: clang/test/CodeGen/matrix-cast.c:39
+
+ f = (fx5x5)i;
+}
----------------
SaurabhJha wrote:
> I tried adding a float -> int conversion too but it failed because of this
> assertion
> https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGExprScalar.cpp#L1339-L1344
> Hopefully that's intended.
Clang should never run into an assertion. If that should not be allowed, then
Clang should emit an error during Sema. I'm not sure why there is such a
restriction for vector types, but I am not sure that this shouldn't be allowed
for matrixes. Perhaps @rjmccall has more thoughts on this, but conversion
between scalar ints to floats is allowed AFAIKT.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99037/new/
https://reviews.llvm.org/D99037
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
