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

Wed Apr 7 03:36:41 PDT 2021

fhahn added a comment.

In D99037#2673367 <https://reviews.llvm.org/D99037#2673367>, @SaurabhJha wrote:

> Hey Florian and John,
>
> Thanks for your reviews so far. Just checked the build. Addressed all previous comments and the build is looking good too except for one thing. For open cl tests, it is failing with this curious `error: initializer element is not a compile-time constant` which I am still not sure how to debug. I tried to debug `Clang.SemaOpenCL::sampler_t_overload.cl` by moving the definition of `MatrixCast` in `OperationKinds.def` to below the definition of `IntToOCLSampler`. That made it work for `sampler_overload` but then the new matrix cast tests started failing.
>
> Will give it another shot today. If you have any thoughts on this, let me know.

I think the issue might be that adding this additional cast-kind caused the value to exceed the maximum supported by the `CastExprBitfields`; the bitfield can only store 64 values, but after adding `MatrixCast`, `CK_IntToOCLSampler` will have value `64`, so assigning to the bitfield results in `0` being assigned. I *think* you have to bump the bitfield size to 7 or perhaps 8 (which may result in slightly better codegen). https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/Stmt.h#L521

================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1199

+Value *ScalarExprEmitter::EmitCastBetweenScalarTypes(
+    Value *Src, QualType SrcType, QualType DstType, llvm::Type *SrcTy,
----------------
Can you put up a patch that just moves the existing code to the function, without adding the matrix specifics? Hopefully that would reduce the diff and make it easy to see that the non-matrix code paths are unchanged?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037