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

Fri Apr 2 13:08:45 PDT 2021

rjmccall 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:
> SaurabhJha wrote:
> > 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.
> Oh right, I think I see what's going on. I think the code you are referring to is specifically for the vector types in Clang, but it is checking the LLVM type and we also use LLVM IR vector types for matrix values, so the code accidentally also covers matrix values at the moment. 
> 
> I *think* we should probably handled matrix values separately in this function. @rjmccall what do you think?
Yes, I would recommend that you recognize matrix values by their Clang type and then call something in the matrix IR-generation library.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99037/new/

https://reviews.llvm.org/D99037