SaurabhJha added inline comments.
================ 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 ---------------- fhahn wrote: > 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. This is happening because we are always passing inSigned argument to `Builder.CreateIntCast` as false here https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGExprScalar.cpp#L1348. IfI change it to `true`, it generates `sext` instructions. I am figuring out how can I determine sign of src and dest type. Because `SrcTy` and `DestTy` are vectors here, they are unsigned and `SrcElementTy` and `DestElementTy` have no method like `isSignedIntegerOrEnumerationType`. Once that's solved, this should be fixed. ================ Comment at: clang/test/CodeGen/matrix-cast.c:39 + + f = (fx5x5)i; +} ---------------- fhahn wrote: > 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. I can probably try removing that assert and see if some other unit or lit tests are failing. We can then make a decision. 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