-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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">; | ||||||||||||||||||||||
def err_typecheck_subscript_out_of_bounds : Error< | ||||||||||||||||||||||
"%select{row|column}0 subscripted is out of bounds">; | ||||||||||||||||||||||
Comment on lines
+7504
to
+7507
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
def err_subscript_function_type : Error< | ||||||||||||||||||||||
"subscript of pointer to function type %0">; | ||||||||||||||||||||||
def err_subscript_incomplete_or_sizeless_type : Error< | ||||||||||||||||||||||
|
@@ -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 " | ||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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. | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
||
|
@@ -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()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -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" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -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(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Would There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair. I was using |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bool IsMaccessor = Name.consume_front("_m"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// consume numeric accessor second so if _m exist we don't consume _ to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// 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])) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Row & Col are unsigned, so this'll underflow rather than return -1. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Also the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if we need separate There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Comment on lines
+4351
to
+4376
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
MemberLoc); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bool SemaHLSL::handleInitialization(VarDecl *VDecl, Expr *&Init) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const HLSLVkConstantIdAttr *ConstIdAttr = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
VDecl->getAttr<HLSLVkConstantIdAttr>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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))); | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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; | ||||||||||||||||||||||||||
} |
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; | ||
} |
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please include tests for incomplete access ( There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
|
||
A._m12 = 3.14; // write is OK | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.