address pr comments

Author: farzonl

clang/lib/CodeGen/CGExprScalar.cpp

@@ -2429,7 +2429,7 @@ static Value *EmitHLSLElementwiseCast(CodeGenFunction &CGF, LValue SrcVal,
 
     llvm::Value *V =
         CGF.Builder.CreateLoad(CGF.CreateIRTemp(DestTy, "flatcast.tmp"));
-    // write to V.
+    // V is an allocated temporary to build the truncated matrix into.
     for (unsigned I = 0, E = MatTy->getNumElementsFlattened(); I < E; I++) {
       unsigned ColMajorIndex =
           (I % MatTy->getNumRows()) * MatTy->getNumColumns() +
@@ -2977,27 +2977,23 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
     return Builder.CreateExtractElement(Vec, Zero, "cast.vtrunc");
   }
   case CK_HLSLMatrixTruncation: {
-    assert((DestTy->isMatrixType() || DestTy->isBuiltinType()) &&
+    assert((DestTy->isConstantMatrixType() || DestTy->isBuiltinType()) &&
            "Destination type must be a matrix or builtin type.");
     Value *Mat = Visit(E);
-    if (auto *MatTy = DestTy->getAs<ConstantMatrixType>()) {
-      SmallVector<int> Mask;
-      unsigned NumCols = MatTy->getNumColumns();
-      unsigned NumRows = MatTy->getNumRows();
-      unsigned ColOffset = NumCols;
-      if (auto *SrcMatTy = E->getType()->getAs<ConstantMatrixType>())
-        ColOffset = SrcMatTy->getNumColumns();
-      for (unsigned R = 0; R < NumRows; R++) {
-        for (unsigned C = 0; C < NumCols; C++) {
-          unsigned I = R * ColOffset + C;
-          Mask.push_back(I);
-        }
+    auto *MatTy = DestTy->getAs<ConstantMatrixType>();
+    SmallVector<int> Mask;
+    unsigned NumCols = MatTy->getNumColumns();
+    unsigned NumRows = MatTy->getNumRows();
+    unsigned ColOffset = NumCols;
+    if (auto *SrcMatTy = E->getType()->getAs<ConstantMatrixType>())
+      ColOffset = SrcMatTy->getNumColumns();
+    for (unsigned R = 0; R < NumRows; R++) {
+      for (unsigned C = 0; C < NumCols; C++) {
+        unsigned I = R * ColOffset + C;
+        Mask.push_back(I);
       }
-
-      return Builder.CreateShuffleVector(Mat, Mask, "trunc");
     }
-    llvm::Value *Zero = llvm::Constant::getNullValue(CGF.SizeTy);
-    return Builder.CreateExtractElement(Mat, Zero, "cast.mtrunc");
+    return Builder.CreateShuffleVector(Mat, Mask, "trunc");
   }
   case CK_HLSLElementwiseCast: {
     RValue RV = CGF.EmitAnyExpr(E);

clang/lib/Sema/SemaChecking.cpp

@@ -12571,9 +12571,10 @@ void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,
       if (SourceMgr.isInSystemMacro(CC))
         return;
       return DiagnoseImpCast(*this, E, T, CC, diag::warn_impcast_vector_scalar);
-    } else if (getLangOpts().HLSL &&
-               Target->castAs<VectorType>()->getNumElements() <
-                   Source->castAs<VectorType>()->getNumElements()) {
+    }
+    if (getLangOpts().HLSL &&
+        Target->castAs<VectorType>()->getNumElements() <
+            Source->castAs<VectorType>()->getNumElements()) {
       // Diagnose vector truncation but don't return. We may also want to
       // diagnose an element conversion.
       DiagnoseImpCast(*this, E, T, CC,
@@ -12593,12 +12594,12 @@ void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,
     Target = VecTy->getElementType().getTypePtr();
 
   if (isa<ConstantMatrixType>(Source)) {
-    if (!isa<ConstantMatrixType>(Target)) {
+    if (Target->isScalarType())
       return DiagnoseImpCast(*this, E, T, CC, diag::warn_impcast_matrix_scalar);
-    } else if (getLangOpts().HLSL &&
-               Target->castAs<ConstantMatrixType>()->getNumElementsFlattened() <
-                   Source->castAs<ConstantMatrixType>()
-                       ->getNumElementsFlattened()) {
+
+    if (getLangOpts().HLSL &&
+        Target->castAs<ConstantMatrixType>()->getNumElementsFlattened() <
+            Source->castAs<ConstantMatrixType>()->getNumElementsFlattened()) {
       // Diagnose Matrix truncation but don't return. We may also want to
       // diagnose an element conversion.
       DiagnoseImpCast(*this, E, T, CC,

clang/test/SemaHLSL/MatrixElementOverloadResolution.hlsl

@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wconversion -verify -o - %s -DERROR=1
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wno-conversion -ast-dump %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -verify -o - %s -DERROR=1
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -ast-dump %s | FileCheck %s
 
 // This test verifies floating point type implicit conversion ranks for overload
 // resolution. In HLSL the built-in type ranks are half < float < double. This

clang/test/SemaHLSL/Types/BuiltinMatrix/MatrixImplicitTruncCastWarnings.hlsl

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-library -finclude-default-header -Wmatrix-conversion -verify %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-library -finclude-default-header -verify %s
 
 export int3x4 trunc_cast(int4x4 i44) {
     int3x4 i34 = i44;