Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions clang/include/clang/Basic/LangOptions.def
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ ENUM_LANGOPT(HLSLVersion, HLSLLangStd, 16, HLSL_Unset, NotCompatible, "HLSL Vers
LANGOPT(HLSLStrictAvailability, 1, 0, NotCompatible,
"Strict availability diagnostic mode for HLSL built-in functions.")
LANGOPT(HLSLSpvUseUnknownImageFormat, 1, 0, NotCompatible, "For storage images and texel buffers, sets the default format to 'Unknown' when not specified via the `vk::image_format` attribute. If this option is not used, the format is inferred from the resource's data type.")
VALUE_LANGOPT(MaxMatrixDimension, 32, (1 << 20) - 1, NotCompatible, "maximum allowed matrix dimension")
Copy link
Member Author

@farzonl farzonl Sep 22, 2025

Choose a reason for hiding this comment

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

The goal should likely be to replace the constexpr MaxElementsPerDimension with this language option.

class ConstantMatrixType final : public MatrixType {
protected:
friend class ASTContext;
/// Number of rows and columns.
unsigned NumRows;
unsigned NumColumns;
static constexpr unsigned MaxElementsPerDimension = (1 << 20) - 1;

But that would require updating all of these static constexpr functions with something that takes a SemaPtr

/// Returns true if \p NumElements is a valid matrix dimension.
static constexpr bool isDimensionValid(size_t NumElements) {
return NumElements > 0 && NumElements <= MaxElementsPerDimension;
}
/// Returns the maximum number of elements per dimension.
static constexpr unsigned getMaxElementsPerDimension() {
return MaxElementsPerDimension;
}

For now we will make this an HLSL only language opt and create an issue to make it more generic:

Suggested change
VALUE_LANGOPT(MaxMatrixDimension, 32, (1 << 20) - 1, NotCompatible, "maximum allowed matrix dimension")
VALUE_LANGOPT(HLSLMaxMatrixDimension, 32, 4, NotCompatible, "maximum allowed matrix dimension")

issue #160190


LANGOPT(CUDAIsDevice , 1, 0, NotCompatible, "compiling for CUDA device")
LANGOPT(CUDAAllowVariadicFunctions, 1, 0, NotCompatible, "allowing variadic functions in CUDA device code")
Expand Down
8 changes: 6 additions & 2 deletions clang/lib/Basic/LangOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,12 @@ void LangOptions::setLangDefaults(LangOptions &Opts, Language Lang,
Opts.NamedLoops = Std.isC2y();

Opts.HLSL = Lang == Language::HLSL;
if (Opts.HLSL && Opts.IncludeDefaultHeader)
Includes.push_back("hlsl.h");
if (Opts.HLSL) {
if (Opts.IncludeDefaultHeader)
Includes.push_back("hlsl.h");
// Set maximum matrix dimension to 4 for HLSL
Opts.MaxMatrixDimension = 4;
}

// Set OpenCL Version.
Opts.OpenCL = Std.isOpenCL();
Expand Down
36 changes: 33 additions & 3 deletions clang/lib/Sema/HLSLExternalSemaSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,39 @@ void HLSLExternalSemaSource::defineHLSLMatrixAlias() {
SourceLocation(), ColsParam));
TemplateParams.emplace_back(ColsParam);

auto *ParamList =
TemplateParameterList::Create(AST, SourceLocation(), SourceLocation(),
TemplateParams, SourceLocation(), nullptr);
const unsigned MaxMatDim = SemaPtr->getLangOpts().MaxMatrixDimension;
auto *MaxRow = IntegerLiteral::Create(
AST, llvm::APInt(AST.getIntWidth(AST.IntTy), MaxMatDim), AST.IntTy,
SourceLocation());
auto *MaxCol = IntegerLiteral::Create(
AST, llvm::APInt(AST.getIntWidth(AST.IntTy), MaxMatDim), AST.IntTy,
SourceLocation());

auto *RowsRef = DeclRefExpr::Create(
AST, NestedNameSpecifierLoc(), SourceLocation(), RowsParam,
/*RefersToEnclosingVariableOrCapture*/ false,
DeclarationNameInfo(RowsParam->getDeclName(), SourceLocation()),
AST.IntTy, VK_LValue);
auto *ColsRef = DeclRefExpr::Create(
AST, NestedNameSpecifierLoc(), SourceLocation(), ColsParam,
/*RefersToEnclosingVariableOrCapture*/ false,
DeclarationNameInfo(ColsParam->getDeclName(), SourceLocation()),
AST.IntTy, VK_LValue);

auto *RowsLE = BinaryOperator::Create(AST, RowsRef, MaxRow, BO_LE, AST.BoolTy,
VK_PRValue, OK_Ordinary,
SourceLocation(), FPOptionsOverride());
auto *ColsLE = BinaryOperator::Create(AST, ColsRef, MaxCol, BO_LE, AST.BoolTy,
VK_PRValue, OK_Ordinary,
SourceLocation(), FPOptionsOverride());

auto *RequiresExpr = BinaryOperator::Create(
AST, RowsLE, ColsLE, BO_LAnd, AST.BoolTy, VK_PRValue, OK_Ordinary,
SourceLocation(), FPOptionsOverride());

auto *ParamList = TemplateParameterList::Create(
AST, SourceLocation(), SourceLocation(), TemplateParams, SourceLocation(),
RequiresExpr);
Copy link
Member Author

Choose a reason for hiding this comment

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

@llvm-beanz I had two questions on this commit for you.

  1. First should we be adding the size limits on the alias like I am doing above or should we be doing it on the underlying matrix_type?
  2. Second it was pretty easy to enforce the size limits without a concept. Are you ok with The RequiresExpr that TemplateParameterList takes as our means of enforcing a size limit?

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. First should we be adding the size limits on the alias like I am doing above or should we be doing it on the underlying matrix_type?

I think this probably boils down to the quality of the error messages, and how we would handle larger matrices in the backend. Since we probably don't want to handle larger matrices we should probably restrict matrix_type. If we get better diagnostics from the requirement being on the template, we may want to do that also.

  1. Second it was pretty easy to enforce the size limits without a concept. Are you ok with The RequiresExpr that TemplateParameterList takes as our means of enforcing a size limit?

Requirement expressions are part of C++ concepts, that was my intention for how this would be done. I don't think we need a separately defined ConceptDecl.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'n that case I'm going to take enforcement of size on matrix_type as a change we will do on #160190 since it sounds like the diagnostics would be better to do this on the alias aswell as the underlying type.


IdentifierInfo &II = AST.Idents.get("matrix", tok::TokenKind::identifier);

Expand Down
6 changes: 5 additions & 1 deletion clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,9 @@ uint64_t5x5 mat;
// expected-error@-1 {{unknown type name 'uint64_t5x5'}}

// Note: this one only fails because -fnative-half-type is not set
uint16_t4x4 mat;
uint16_t4x4 mat2;
// expected-error@-1 {{unknown type name 'uint16_t4x4'}}

matrix<int, 5, 5> mat3;
// expected-error@-1 {{constraints not satisfied for alias template 'matrix' [with element = int, rows_count = 5, cols_count = 5]}}
// expected-note@* {{because '5 <= 4' (5 <= 4) evaluated to false}}
4 changes: 2 additions & 2 deletions clang/test/SemaHLSL/BuiltIns/matrix-errors.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

// Some bad declarations
hlsl::matrix ShouldWorkSomeday; // expected-error{{use of alias template 'hlsl::matrix' requires template arguments}}
// expected-note@*:* {{template declaration from hidden source: template <class element = float, int rows_count = 4, int cols_count = 4> using matrix = element __attribute__((matrix_type(rows_count, cols_count)))}}
// expected-note@*:* {{template declaration from hidden source: template <class element = float, int rows_count = 4, int cols_count = 4> requires rows_count <= 4 && cols_count <= 4 using matrix = element __attribute__((matrix_type(rows_count, cols_count)))}}

hlsl::matrix<1,1,1> BadMat; // expected-error{{template argument for template type parameter must be a type}}
// expected-note@*:* {{template parameter from hidden source: class element = float}}
Expand All @@ -11,7 +11,7 @@ hlsl::matrix<int, float,4> AnotherBadMat; // expected-error{{template argument f
// expected-note@*:* {{template parameter from hidden source: int rows_count = 4}}

hlsl::matrix<int, 2, 3, 2> YABV; // expected-error{{too many template arguments for alias template 'matrix'}}
// expected-note@*:* {{template declaration from hidden source: template <class element = float, int rows_count = 4, int cols_count = 4> using matrix = element __attribute__((matrix_type(rows_count, cols_count)))}}
// expected-note@*:* {{template declaration from hidden source: template <class element = float, int rows_count = 4, int cols_count = 4> requires rows_count <= 4 && cols_count <= 4 using matrix = element __attribute__((matrix_type(rows_count, cols_count)))}}

// This code is rejected by clang because clang puts the HLSL built-in types
// into the HLSL namespace.
Expand Down