Skip to content

[HLSL] Add _m and _<numeric> based accessors to hlsl::matrix #163220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[HLSL] Add _m and _<numeric> based accessors to hlsl::matrix #163220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
48d63d3
[HLSL] Add _m and _<numeric> based accessors to hlsl::matrix
farzonl Oct 13, 2025
4091565
update err_typecheck_subscript_not_integer to indicate array or matrix
farzonl Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7500,7 +7500,11 @@ def ext_subscript_non_lvalue : Extension<
def err_typecheck_subscript_value : Error<
"subscripted value is not an array, pointer, or vector">;
def err_typecheck_subscript_not_integer : Error<
"array subscript is not an integer">;
"%select{array|matrix}0 subscript is not an integer">;
def err_typecheck_subscript_not_in_bounds : Error<
"%select{row|column}0 subscript is out of bounds of %select{zero|one}1 based indexing">;
Comment on lines +7504 to +7505
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an HLSL-specific error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. i can add hlsl to the err name. also still working out what the text should be for this. If it should exactly match the dxc string or not.

def err_typecheck_subscript_out_of_bounds : Error<
"%select{row|column}0 subscripted is out of bounds">;
Comment on lines +7504 to +7507
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def err_typecheck_subscript_not_in_bounds : Error<
"%select{row|column}0 subscript is out of bounds of %select{zero|one}1 based indexing">;
def err_typecheck_subscript_out_of_bounds : Error<
"%select{row|column}0 subscripted is out of bounds">;
def err_typecheck_subscript_not_in_bounds : Error<
"matrix %select{row|column}0 subscript is out of bounds of %select{zero|one}1 based indexing">;
def err_typecheck_subscript_out_of_bounds : Error<
"matrix %select{row|column}0 subscript is out of bounds">;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"%select{row|column}0 subscripted is out of bounds">;
"%select{row|column}0 subscript is out of bounds">;

def err_subscript_function_type : Error<
"subscript of pointer to function type %0">;
def err_subscript_incomplete_or_sizeless_type : Error<
Expand Down Expand Up @@ -12972,6 +12976,7 @@ def err_builtin_matrix_stride_too_small: Error<
"stride must be greater or equal to the number of rows">;
def err_builtin_matrix_invalid_dimension: Error<
"%0 dimension is outside the allowed range [1, %1]">;
def err_builtin_matrix_invalid_member: Error<"invalid matrix member">;

def warn_mismatched_import : Warning<
"import %select{module|name}0 (%1) does not match the import %select{module|name}0 (%2) of the "
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Sema/SemaHLSL.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ class SemaHLSL : public SemaBase {
bool transformInitList(const InitializedEntity &Entity, InitListExpr *Init);
bool handleInitialization(VarDecl *VDecl, Expr *&Init);
void deduceAddressSpace(VarDecl *Decl);
ExprResult tryBuildHLSLMatrixElementAccessor(Expr *Base,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ExprResult tryBuildHLSLMatrixElementAccessor(Expr *Base,
ExprResult tryBuildMatrixElementAccessor(Expr *Base,

There is already HLSL in class name SemaHLSL.

SourceLocation MemberLoc,
const IdentifierInfo *MemberId);

private:
// HLSL resource type attributes need to be processed all at once.
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5334,7 +5334,7 @@ Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, SourceLocation LLoc,
// C99 6.5.2.1p1
if (!IndexExpr->getType()->isIntegerType() && !IndexExpr->isTypeDependent())
return ExprError(Diag(LLoc, diag::err_typecheck_subscript_not_integer)
<< IndexExpr->getSourceRange());
<< /*array*/ 0 << IndexExpr->getSourceRange());

if ((IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_S) ||
IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_U)) &&
Expand Down Expand Up @@ -16261,7 +16261,7 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
!Idx->getType()->isIntegerType())
return ExprError(
Diag(Idx->getBeginLoc(), diag::err_typecheck_subscript_not_integer)
<< Idx->getSourceRange());
<< /*array*/ 0 << Idx->getSourceRange());

// Record this array index.
Comps.push_back(OffsetOfNode(OC.LocStart, Exprs.size(), OC.LocEnd));
Expand Down
20 changes: 14 additions & 6 deletions clang/lib/Sema/SemaExprMember.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "clang/Sema/Overload.h"
#include "clang/Sema/Scope.h"
#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/SemaHLSL.h"
#include "clang/Sema/SemaObjC.h"
#include "clang/Sema/SemaOpenMP.h"

Expand Down Expand Up @@ -1669,12 +1670,19 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,

// HLSL supports implicit conversion of scalar types to single element vector
// rvalues in member expressions.
if (S.getLangOpts().HLSL && BaseType->isScalarType()) {
QualType VectorTy = S.Context.getExtVectorType(BaseType, 1);
BaseExpr = S.ImpCastExprToType(BaseExpr.get(), VectorTy, CK_VectorSplat,
BaseExpr.get()->getValueKind());
return LookupMemberExpr(S, R, BaseExpr, IsArrow, OpLoc, SS, ObjCImpDecl,
HasTemplateArgs, TemplateKWLoc);
if (S.getLangOpts().HLSL) {
if (BaseType->isScalarType()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the comment from lines 1671-2 here.

QualType VectorTy = S.Context.getExtVectorType(BaseType, 1);
BaseExpr = S.ImpCastExprToType(BaseExpr.get(), VectorTy, CK_VectorSplat,
BaseExpr.get()->getValueKind());
return LookupMemberExpr(S, R, BaseExpr, IsArrow, OpLoc, SS, ObjCImpDecl,
HasTemplateArgs, TemplateKWLoc);
}
if (!IsArrow && BaseType->isConstantMatrixType()) {
if (const auto *II = MemberName.getAsIdentifierInfo())
return S.HLSL().tryBuildHLSLMatrixElementAccessor(BaseExpr.get(),
MemberLoc, II);
}
}

S.Diag(OpLoc, diag::err_typecheck_member_reference_struct_union)
Expand Down
97 changes: 97 additions & 0 deletions clang/lib/Sema/SemaHLSL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "clang/AST/Expr.h"
#include "clang/AST/HLSLResource.h"
#include "clang/AST/Type.h"
#include "clang/AST/TypeBase.h"
#include "clang/AST/TypeLoc.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/DiagnosticSema.h"
Expand All @@ -31,6 +32,7 @@
#include "clang/Basic/TargetInfo.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Ownership.h"
#include "clang/Sema/ParsedAttr.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/Template.h"
Expand Down Expand Up @@ -4313,6 +4315,101 @@ bool SemaHLSL::transformInitList(const InitializedEntity &Entity,
return true;
}

ExprResult SemaHLSL::tryBuildHLSLMatrixElementAccessor(
Expr *Base, SourceLocation MemberLoc, const IdentifierInfo *MemberId) {
if (!Base)
return ExprError();

QualType T = Base->getType();
const ConstantMatrixType *MT = T->getAs<ConstantMatrixType>();
if (!MT)
return ExprError();

auto parseHLSLMatrixAccessor =
[this, MemberLoc, MemberId](unsigned &Row, unsigned &Col) -> ExprResult {
StringRef Name = MemberId->getNameStart();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
StringRef Name = MemberId->getNameStart();
StringRef Name = MemberId->getName();

Would getName() be better here? It returns StringRef directly while getNameStart() returns zero-terminated const char *.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair. I was using getNameStart because thats what vector_ext were using.

bool IsMaccessor = Name.consume_front("_m");
// consume numeric accessor second so if _m exist we don't consume _ to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// consume numeric accessor second so if _m exist we don't consume _ to
// consume numeric accessor second so if _m exist we don't consume _ too

// early.
bool IsNumericAccessor = Name.consume_front("_");
if (!IsMaccessor && !IsNumericAccessor)
return ExprError(
Diag(MemberLoc, diag::err_builtin_matrix_invalid_member));

auto isDigit = [](char c) { return c >= '0' && c <= '9'; };
auto isZeroBasedIndex = [](char c) { return c >= '0' && c <= '3'; };
auto isOneBasedIndex = [](char c) { return c >= '1' && c <= '4'; };
if (Name.empty() || !isDigit(Name[0]) || !isDigit(Name[1])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name[1] will index beyond the string if the index is complete (_m0). And too many indices are not diagnosed (_0123).

return ExprError(
Diag(MemberLoc, diag::err_typecheck_subscript_not_integer)
<< /*matrix*/ 1);
}

Row = Name[0] - '0';
Col = Name[1] - '0';
bool HasIndexingError = false;
if (IsNumericAccessor) {
Row--;
Col--;
// Note: we add diagnostic errors here because otherwise we will return -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Row & Col are unsigned, so this'll underflow rather than return -1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case where it would be -1 we emit an expression error. the -1 will never make it to be consumed by CreateBuiltinMatrixSubscriptExpr let alone llvm::APInt.

Also the RowIdx and ColIdx values to MatrixSubscriptExpr are suppose to be Context.UnsignedIntTy. Thats how the other uses have been doing it. And is how I set it up when I do IntegerLiteral::Create. I think it should be fine to keep this unsigned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't suggesting we change the type of the variable. Just that it is inaccurate to say that we'll return -1, because that's not a value than can be expressed in an unsigned.

if (!isOneBasedIndex(Name[0])) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*row*/ 0 << /*one*/ 1;
HasIndexingError = true;
}
if (!isOneBasedIndex(Name[1])) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*col*/ 1 << /*one*/ 1;
HasIndexingError = true;
}
} else {
if (!isZeroBasedIndex(Name[0])) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*row*/ 0 << /*zero*/ 0;
HasIndexingError = true;
}
if (!isZeroBasedIndex(Name[1])) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*col*/ 1 << /*zero*/ 0;
HasIndexingError = true;
}
}
Comment on lines +4351 to +4376
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the language should be on these errors but DXC today has a way of erroring if you are out of bounds of the indexing scheme

I could exactly match this:
https://godbolt.org/z/nWvf5EM47

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we need separate *not_in_bounds vs. *out_of_bounds errors. Maybe parseHLSLMatrixAccessor could take the actual matrix dimensions and check directly against them? I like the *not_in_bounds version of the error better because it includes what kind of indexing it is (0-based or 1-based).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking actual matrix dimensions is needed because we can't do correct codegen unless we know it. Checking index base is also needed because of accessor restrictions. They do overlap, but given the potential of user misuse of ._m vs ._ it is important to let users know which one is 0-based vs 1-based. How we present that information is the real question here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I was just suggesting that we could check all of these in one step if parseHLSLMatrixAccessor is aware of the actual dimensions, including letting users know if the indexing is 0-based or 1-based.

Comment on lines +4351 to +4376
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (IsNumericAccessor) {
Row--;
Col--;
// Note: we add diagnostic errors here because otherwise we will return -1
if (!isOneBasedIndex(Name[0])) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*row*/ 0 << /*one*/ 1;
HasIndexingError = true;
}
if (!isOneBasedIndex(Name[1])) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*col*/ 1 << /*one*/ 1;
HasIndexingError = true;
}
} else {
if (!isZeroBasedIndex(Name[0])) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*row*/ 0 << /*zero*/ 0;
HasIndexingError = true;
}
if (!isZeroBasedIndex(Name[1])) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*col*/ 1 << /*zero*/ 0;
HasIndexingError = true;
}
}
if (IsNumericAccessor) {
Row--;
Col--;
}
// Note: we add diagnostic errors here because otherwise we will return -1
for (unsigned I = 0; I < 2; I++) {
bool IsValid = IsNumericAccessor ? isOneBasedIndex(Name[I]) : isZeroBasedIndex(Name[I]);
if (!IsValid) {
Diag(MemberLoc, diag::err_typecheck_subscript_not_in_bounds)
<< /*row*/ I << /*one*/ (unsigned)IsNumericAccessor;
HasIndexingError = true;
}
}

Just a suggestion - the code seems to be very repetitive.

if (HasIndexingError)
return ExprError();
return ExprEmpty();
};
unsigned Row = 0, Col = 0;
ExprResult ParseResult = parseHLSLMatrixAccessor(Row, Col);
if (ParseResult.isInvalid())
return ParseResult;

unsigned Rows = MT->getNumRows();
unsigned Cols = MT->getNumColumns();
bool HasBoundsError = false;
if (Row >= Rows) {
Diag(MemberLoc, diag::err_typecheck_subscript_out_of_bounds) << /*Row*/ 0;
HasBoundsError = true;
}
if (Col >= Cols) {
Diag(MemberLoc, diag::err_typecheck_subscript_out_of_bounds) << /*Col*/ 1;
HasBoundsError = true;
}
if (HasBoundsError)
return ExprError();

auto mkIdx = [&](unsigned v) -> Expr * {
ASTContext &Context = SemaRef.getASTContext();
return IntegerLiteral::Create(Context, llvm::APInt(32, v),
Context.UnsignedIntTy, MemberLoc);
};
Expr *RowIdx = mkIdx(Row);
Expr *ColIdx = mkIdx(Col);

// Build A[Row][Col], reusing the existing matrix subscript machinery.
return SemaRef.CreateBuiltinMatrixSubscriptExpr(Base, RowIdx, ColIdx,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other problem with using a subscript expression here is that the AST won't actually retain source information accurately enough to drive AST-driven tooling. I would expect a rewriter with this AST representation to rewrite A._m00 to A[0][0]. The AST node needs to know the indices and the spelling in order to be able to recreate the original syntax without referring back to the source.

MemberLoc);
}

bool SemaHLSL::handleInitialization(VarDecl *VDecl, Expr *&Init) {
const HLSLVkConstantIdAttr *ConstIdAttr =
VDecl->getAttr<HLSLVkConstantIdAttr>();
Expand Down
39 changes: 39 additions & 0 deletions clang/test/AST/HLSL/matrix-member-access.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s

typedef float float3x3 __attribute__((matrix_type(3,3)));
Copy link
Member

@hekota hekota Oct 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to use using asymmetrical matrix and subscripts to make sure rows don't get mixed up with columns.


[numthreads(1,1,1)]
void ok() {
float3x3 A;
// CHECK: BinaryOperator 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:{{[0-9]+}}, col:{{[0-9]+}}> 'float' lvalue matrixcomponent '='
// CHECK-NEXT: MatrixSubscriptExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'float' lvalue matrixcomponent
// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'float3x3':'matrix<float, 3, 3>' lvalue Var 0x{{[0-9a-fA-F]+}} 'A' 'float3x3':'matrix<float, 3, 3>'
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 1
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 2
// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'float' 3.140000e+00
Comment on lines +8 to +13
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// CHECK: BinaryOperator 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:{{[0-9]+}}, col:{{[0-9]+}}> 'float' lvalue matrixcomponent '='
// CHECK-NEXT: MatrixSubscriptExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'float' lvalue matrixcomponent
// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'float3x3':'matrix<float, 3, 3>' lvalue Var 0x{{[0-9a-fA-F]+}} 'A' 'float3x3':'matrix<float, 3, 3>'
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 1
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 2
// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'float' 3.140000e+00
// CHECK: BinaryOperator {{.*}} 'float' lvalue matrixcomponent '='
// CHECK-NEXT: MatrixSubscriptExpr {{.*}} 'float' lvalue matrixcomponent
// CHECK-NEXT: DeclRefExpr {{.*}} 'float3x3':'matrix<float, 3, 3>' lvalue Var {{.*}} 'A' 'float3x3':'matrix<float, 3, 3>'
// CHECK-NEXT: IntegerLiteral {{.*}} 'unsigned int' 1
// CHECK-NEXT: IntegerLiteral {{.*}} 'unsigned int' 2
// CHECK-NEXT: FloatingLiteral {{.*}} 'float' 3.140000e+00

If you don't care about the line & column values, the check lines can be simplified to be easier to read.

A._m12 = 3.14;

// CHECK: VarDecl 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> col:{{[0-9]+}} r 'float' cinit
// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'float' <LValueToRValue>
// CHECK-NEXT: MatrixSubscriptExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'float' lvalue matrixcomponent
// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'float3x3':'matrix<float, 3, 3>' lvalue Var 0x{{[0-9a-fA-F]+}} 'A' 'float3x3':'matrix<float, 3, 3>'
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 0
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 0
float r = A._m00;

// CHECK: VarDecl 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> col:{{[0-9]+}} good1 'float' cinit
// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'float' <LValueToRValue>
// CHECK-NEXT: MatrixSubscriptExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'float' lvalue matrixcomponent
// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'float3x3':'matrix<float, 3, 3>' lvalue Var 0x{{[0-9a-fA-F]+}} 'A' 'float3x3':'matrix<float, 3, 3>'
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 0
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 0
float good1 = A._11;

// CHECK: VarDecl 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> col:{{[0-9]+}} good2 'float' cinit
// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'float' <LValueToRValue>
// CHECK-NEXT: MatrixSubscriptExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'float' lvalue matrixcomponent
// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'float3x3':'matrix<float, 3, 3>' lvalue Var 0x{{[0-9a-fA-F]+}} 'A' 'float3x3':'matrix<float, 3, 3>'
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 2
// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <col:{{[0-9]+}}> 'unsigned int' 2
float good2 = A._33;
}
25 changes: 25 additions & 0 deletions clang/test/CodeGenHLSL/matrix-member-access.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// RUN: %clang_cc1 -std=hlsl202x -finclude-default-header -x hlsl -triple \
// RUN: dxil-pc-shadermodel6.3-library %s -fnative-half-type \
// RUN: -emit-llvm -disable-llvm-passes -o - | FileCheck %s


half3x3 write_pi(half3x3 A) {
//CHECK: [[MAT_INS:%.*]] = insertelement <9 x half> %{{[0-9]+}}, half 0xH4248, i32 7
//CHECK-NEXT: store <9 x half> [[MAT_INS]], ptr %{{.*}}, align 2
A._m12 = 3.14;
return A;
}

half read_1x1(half3x3 A) {
//CHECK: [[MAT_EXT:%.*]] = extractelement <9 x half> %{{[0-9]+}}, i32 0
return A._11;
}
half read_m0x0(half3x3 A) {
//CHECK: [[MAT_EXT:%.*]] = extractelement <9 x half> %{{[0-9]+}}, i32 0
return A._m00;
}

half read_3x3(half3x3 A) {
//CHECK: [[MAT_EXT:%.*]] = extractelement <9 x half> %{{[0-9]+}}, i32 8
return A._33;
}
20 changes: 20 additions & 0 deletions clang/test/SemaHLSL/matrix-member-access.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -finclude-default-header -verify %s

void foo() {
float3x3 A;
float r = A._m00; // read is ok
float good1 = A._11;
float good2 = A._33;

float bad0 = A._m44; // expected-error {{row subscript is out of bounds of zero based indexing}} expected-error {{column subscript is out of bounds of zero based indexing}}
float bad1 = A._m33; // expected-error {{row subscripted is out of bounds}} expected-error {{column subscripted is out of bounds}}
float bad2 = A._mA2; // expected-error {{matrix subscript is not an integer}}
float bad3 = A._m2F; // expected-error {{matrix subscript is not an integer}}

float bad4 = A._00; // expected-error {{row subscript is out of bounds of one based indexing}} expected-error {{column subscript is out of bounds of one based indexing}}
float bad5 = A._44; // expected-error {{row subscripted is out of bounds}} expected-error {{column subscripted is out of bounds}}
float bad6 = A._55; // expected-error {{row subscript is out of bounds of one based indexing}} expected-error {{column subscript is out of bounds of one based indexing}}
Comment on lines +9 to +16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include tests for incomplete access (A._m1) and too many indices (A._m12345). It might also be good if some of these were testing that just one dimension is out of bounds. Also an asymmetrical case where column & row order is mixed up (i.e. that accessing ._23 on float2x3 is ok but ._32 is not).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at dxc we need a new error to cover the too few\many indicies case https://godbolt.org/z/o6PGjdYe3
error: invalid format for matrix subscript.



A._m12 = 3.14; // write is OK
}