PR feedback improvements

Author: farzonl

clang/include/clang/AST/Expr.h

@@ -2807,8 +2807,7 @@ class MatrixSingleSubscriptExpr : public Expr {
                             SourceLocation RBracketLoc)
       : Expr(MatrixSingleSubscriptExprClass, T,
              Base->getValueKind(), // lvalue/rvalue follows the matrix base
-             OK_MatrixComponent) { // or OK_Ordinary/OK_VectorComponent if you
-                                   // prefer
+             OK_MatrixComponent) {
     SubExprs[BASE] = Base;
     SubExprs[ROW_IDX] = RowIdx;
     ArrayOrMatrixSubscriptExprBits.RBracketLoc = RBracketLoc;

clang/include/clang/Basic/Specifiers.h

@@ -165,7 +165,7 @@ namespace clang {
     /// Objective-C method calls.
     OK_ObjCSubscript,
 
-    /// A matrix component is a single element of a matrix.
+    /// A matrix component is a single element or range of elements on a matrix.
     OK_MatrixComponent
   };
 

clang/lib/CodeGen/CGExpr.cpp

@@ -2473,22 +2473,18 @@ RValue CodeGenFunction::EmitLoadOfLValue(LValue LV, SourceLocation Loc) {
     unsigned NumCols = MT->getNumColumns();
 
     llvm::Value *MatrixVec = EmitLoadOfScalar(LV, Loc);
-
     llvm::Value *Row = LV.getMatrixRowIdx();
-    llvm::Value *Result =
-        llvm::PoisonValue::get(ConvertType(LV.getType())); // <NumCols x T>
+    llvm::Type *ElemTy = ConvertType(MT->getElementType());
+    llvm::Type *RowTy = llvm::FixedVectorType::get(ElemTy, MT->getNumColumns());
+    llvm::Value *Result = llvm::PoisonValue::get(RowTy); // <NumCols x T>
 
     llvm::MatrixBuilder MB(Builder);
 
     for (unsigned Col = 0; Col < NumCols; ++Col) {
       llvm::Value *ColIdx = llvm::ConstantInt::get(Row->getType(), Col);
-
       llvm::Value *EltIndex = MB.CreateIndex(Row, ColIdx, NumRows);
-
       llvm::Value *Elt = Builder.CreateExtractElement(MatrixVec, EltIndex);
-
       llvm::Value *Lane = llvm::ConstantInt::get(Builder.getInt32Ty(), Col);
-
       Result = Builder.CreateInsertElement(Result, Elt, Lane);
     }
 
@@ -4981,9 +4977,9 @@ LValue CodeGenFunction::EmitMatrixSingleSubscriptExpr(
 
     unsigned Row = RowConst->getZExtValue();
     unsigned Start = Row * NumCols;
-    for (unsigned C = 0; C < NumCols; ++C) {
+    for (unsigned C = 0; C < NumCols; ++C)
       Indices.push_back(llvm::ConstantInt::get(Int32Ty, Start + C));
-    }
+
     llvm::Constant *Elts = llvm::ConstantVector::get(Indices);
     return LValue::MakeExtVectorElt(
         MaybeConvertMatrixAddress(Base.getAddress(), *this), Elts,

clang/lib/CodeGen/CGValue.h

@@ -200,6 +200,9 @@ class LValue {
     // Index into a vector subscript: V[i]
     llvm::Value *VectorIdx;
 
+    // Index into a matrix row subscript: M[i]
+    llvm::Value *MatrixRowIdx;
+
     // ExtVector element subset: V.xyx
     llvm::Constant *VectorElts;
 
@@ -402,7 +405,7 @@ class LValue {
 
   llvm::Value *getMatrixRowIdx() const {
     assert(isMatrixRow());
-    return VectorIdx;
+    return MatrixRowIdx;
   }
 
   // extended vector elements.
@@ -498,7 +501,7 @@ class LValue {
                               TBAAAccessInfo TBAAInfo) {
     LValue LV;
     LV.LVType = MatrixRow;
-    LV.VectorIdx = RowIdx; // store the row index here
+    LV.MatrixRowIdx = RowIdx; // store the row index here
     LV.Initialize(MatrixTy, MatrixTy.getQualifiers(), Addr, BaseInfo, TBAAInfo);
     return LV;
   }

clang/lib/Sema/SemaExpr.cpp

@@ -5160,8 +5160,6 @@ ExprResult Sema::CreateBuiltinMatrixSubscriptExpr(Expr *Base, Expr *RowIdx,
   RowIdx = RowR.get();
 
   if (!ColumnIdx) {
-    if (getLangOpts().HLSL)
-      return CreateBuiltinMatrixSingleSubscriptExpr(Base, RowIdx, RBLoc);
     return new (Context) MatrixSubscriptExpr(
         Base, RowIdx, ColumnIdx, Context.IncompleteMatrixIdxTy, RBLoc);
   }
@@ -21641,12 +21639,17 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) {
     return ExprError();
   }
 
-  case BuiltinType::IncompleteMatrixIdx:
-    Diag(cast<MatrixSubscriptExpr>(E->IgnoreParens())
-             ->getRowIdx()
-             ->getBeginLoc(),
-         diag::err_matrix_incomplete_index);
+  case BuiltinType::IncompleteMatrixIdx: {
+    auto *MS = cast<MatrixSubscriptExpr>(E->IgnoreParens());
+    // At this point, we know there was no second [] to complete the operator.
+    // In HLSL, treat "m[row]" as selecting a row lane of column sized vector.
+    if (getLangOpts().HLSL) {
+      return CreateBuiltinMatrixSingleSubscriptExpr(
+          MS->getBase(), MS->getRowIdx(), E->getExprLoc());
+    }
+    Diag(MS->getRowIdx()->getBeginLoc(), diag::err_matrix_incomplete_index);
     return ExprError();
+  }
 
   // Expressions of unknown type.
   case BuiltinType::ArraySection:

clang/test/AST/HLSL/matrix-single-subscript-getter.hlsl

@@ -48,7 +48,7 @@ export float4 AddFloatMatrixConstant(float4x4 M) {
 // CHECK-NEXT: MatrixSingleSubscriptExpr {{.*}} 'vector<float, 4>' lvalue matrixcomponent
 // CHECK-NEXT: DeclRefExpr {{.*}} 'float4x4':'matrix<float, 4, 4>' lvalue ParmVar {{.*}} 'M' 'float4x4':'matrix<float, 4, 4>'
 // CHECK-NEXT: IntegerLiteral {{.*}} 'int' 3
-    return M[0] + M[1] + M[2] +M[3];
+    return M[0] + M[1] + M[2] + M[3];
 }
 
 export int4 AddIntMatrixConstant(int4x4 M) {
@@ -73,5 +73,5 @@ export int4 AddIntMatrixConstant(int4x4 M) {
 // CHECK-NEXT: MatrixSingleSubscriptExpr {{.*}} 'vector<int, 4>' lvalue matrixcomponent
 // CHECK-NEXT: DeclRefExpr {{.*}} 'int4x4':'matrix<int, 4, 4>' lvalue ParmVar {{.*}} 'M' 'int4x4':'matrix<int, 4, 4>'
 // CHECK-NEXT: IntegerLiteral {{.*}} 'int' 3
-   return M[0] + M[1] + M[2] +M[3];
-}
+   return M[0] + M[1] + M[2] + M[3];
+}

clang/test/AST/HLSL/matrix-single-subscript-setter.hlsl

@@ -56,4 +56,4 @@ export void setMatrixConstIndex(out int4x4 M, int4x4 N ) {
 // CHECK-NEXT: DeclRefExpr {{.*}} 'int4x4':'matrix<int, 4, 4>' lvalue ParmVar {{.*}} 'N' 'int4x4':'matrix<int, 4, 4>'
 // CHECK-NEXT: IntegerLiteral {{.*}} 'int' 0
     M[3] = N[0];
-}
+}

clang/test/AST/HLSL/matrix-single-subscript-swizzle.hlsl

@@ -53,4 +53,4 @@ export float3 getMatrix(float4x4 M, int index) {
 // CHECK-NEXT: DeclRefExpr {{.*}} 'float4x4':'matrix<float, 4, 4>' lvalue ParmVar {{.*}} 'M' 'float4x4':'matrix<float, 4, 4>'
 // CHECK-NEXT: DeclRefExpr {{.*}} 'int' lvalue ParmVar {{.*}} 'index' 'int'
     return M[index].rgb;
-}
+}

clang/test/CodeGenHLSL/BasicFeatures/MatrixSingleSubscriptDynamicSwizzle.hlsl

@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.7-library -disable-llvm-passes -emit-llvm -finclude-default-header -o - %s | FileCheck %s
+// BUG: https://github.com/llvm/llvm-project/issues/170777
 // XFAIL: *
 
 void setMatrix(out float4x4 M, int index, float4 V) {

clang/test/CodeGenHLSL/BasicFeatures/MatrixSingleSubscriptGetter.hlsl

@@ -151,7 +151,7 @@ int4 getIntMatrixDynamic(int4x4 M, int index) {
 // CHECK-NEXT:    ret <4 x float> [[ADD32]]
 //
 float4 AddFloatMatrixConstant(float4x4 M) {
-    return M[0] + M[1] + M[2] +M[3];
+    return M[0] + M[1] + M[2] + M[3];
 }
 
 // CHECK-LABEL: define hidden noundef <4 x i32> @_Z20AddIntMatrixConstantu11matrix_typeILm4ELm4EiE(
@@ -201,5 +201,5 @@ float4 AddFloatMatrixConstant(float4x4 M) {
 // CHECK-NEXT:    ret <4 x i32> [[ADD32]]
 //
 int4 AddIntMatrixConstant(int4x4 M) {
-   return M[0] + M[1] + M[2] +M[3];
+   return M[0] + M[1] + M[2] + M[3];
 }