address pr comments

Author: farzonl

clang/docs/MatrixTypes.rst

@@ -106,7 +106,8 @@ Otherwise, the result is a glvalue with type ``cv T`` and with the same value
 category as ``E1`` which refers to the element at the given row and column in
 the matrix.
 
-Programs containing a single subscript expression into a matrix are ill-formed.
+A single subscript expression into a matrix is legal in HLSL and yields the
+column-sized vector for the selected row, but is ill-formed in C and C++.
 
 **Note**: We considered providing an expression of the form
 ``postfix-expression [expression]`` to access columns of a matrix. We think

clang/lib/CodeGen/CGExpr.cpp

@@ -2729,18 +2729,13 @@ void CodeGenFunction::EmitStoreThroughLValue(RValue Src, LValue Dst,
 
       llvm::Value *Row = Dst.getMatrixRowIdx();
       llvm::Value *RowVal = Src.getScalarVal(); // <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 *Lane = llvm::ConstantInt::get(Builder.getInt32Ty(), Col);
-
         llvm::Value *NewElt = Builder.CreateExtractElement(RowVal, Lane);
-
         MatrixVec = Builder.CreateInsertElement(MatrixVec, NewElt, EltIndex);
       }
 
@@ -4968,7 +4963,6 @@ LValue CodeGenFunction::EmitMatrixSingleSubscriptExpr(
   llvm::Value *RowIdx = EmitMatrixIndexExpr(E->getRowIdx());
 
   if (auto *RowConst = llvm::dyn_cast<llvm::ConstantInt>(RowIdx)) {
-
     // Extract matrix shape from the AST type
     const auto *MatTy = E->getBase()->getType()->castAs<ConstantMatrixType>();
     unsigned NumCols = MatTy->getNumColumns();

clang/lib/CodeGen/CGExprScalar.cpp

@@ -2120,7 +2120,6 @@ Value *ScalarExprEmitter::VisitMatrixSingleSubscriptExpr(
 
   // Row index
   Value *RowIdx = CGF.EmitMatrixIndexExpr(E->getRowIdx());
-
   llvm::MatrixBuilder MB(Builder);
 
   // The row index must be in [0, NumRows)

clang/lib/Sema/SemaExpr.cpp

@@ -5159,10 +5159,9 @@ ExprResult Sema::CreateBuiltinMatrixSubscriptExpr(Expr *Base, Expr *RowIdx,
     return RowR;
   RowIdx = RowR.get();
 
-  if (!ColumnIdx) {
+  if (!ColumnIdx)
     return new (Context) MatrixSubscriptExpr(
         Base, RowIdx, ColumnIdx, Context.IncompleteMatrixIdxTy, RBLoc);
-  }
 
   // Build an unanalyzed expression if any of the operands is type-dependent.
   if (Base->isTypeDependent() || RowIdx->isTypeDependent() ||