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
