[PATCH] D99037: [Matrix] Implement explicit type conversions for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

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