https://github.com/farzonl updated https://github.com/llvm/llvm-project/pull/163307
>From f709052724132fd2a14f0815bd28db89408b7b6a Mon Sep 17 00:00:00 2001 From: Farzon Lotfi <[email protected]> Date: Mon, 13 Oct 2025 21:50:45 -0400 Subject: [PATCH 1/5] [NFC][Matrix][Clang][HLSL] Move MaxMatrixDimension to a LangOpt fixes #160190 This change just makes MaxMatrixDimension configurable by language mode. It was previously introduced in https://github.com/llvm/llvm-project/commit/94b43118e2203fed8ca0377ae762c08189aa6f3d when there was not a need to make dimensions configurable. Current testing to this effect exists in: - clang/test/Sema/matrix-type-builtins.c - clang/test/SemaCXX/matrix-type-builtins.cpp - clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl I considered adding a driver flag to `clang/include/clang/Driver/Options.td` but HLSL matrix max dim is always 4 so we don't need this configurable beyond that size for our use case. --- clang/include/clang/AST/TypeBase.h | 12 ------------ clang/include/clang/Basic/LangOptions.def | 1 + clang/lib/AST/ASTContext.cpp | 4 ++-- clang/lib/Basic/LangOptions.cpp | 8 ++++++-- clang/lib/Sema/HLSLExternalSemaSource.cpp | 3 ++- clang/lib/Sema/SemaChecking.cpp | 4 ++-- clang/lib/Sema/SemaType.cpp | 4 ++-- 7 files changed, 15 insertions(+), 21 deletions(-) diff --git a/clang/include/clang/AST/TypeBase.h b/clang/include/clang/AST/TypeBase.h index 5892566592513..f07861f50fe8c 100644 --- a/clang/include/clang/AST/TypeBase.h +++ b/clang/include/clang/AST/TypeBase.h @@ -4378,8 +4378,6 @@ class ConstantMatrixType final : public MatrixType { unsigned NumRows; unsigned NumColumns; - static constexpr unsigned MaxElementsPerDimension = (1 << 20) - 1; - ConstantMatrixType(QualType MatrixElementType, unsigned NRows, unsigned NColumns, QualType CanonElementType); @@ -4398,16 +4396,6 @@ class ConstantMatrixType final : public MatrixType { return getNumRows() * getNumColumns(); } - /// 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; - } - void Profile(llvm::FoldingSetNodeID &ID) { Profile(ID, getElementType(), getNumRows(), getNumColumns(), getTypeClass()); diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 9e850089ad87f..690439c1258c1 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -432,6 +432,7 @@ ENUM_LANGOPT(RegisterStaticDestructors, RegisterStaticDestructorsKind, 2, LANGOPT(RegCall4, 1, 0, NotCompatible, "Set __regcall4 as a default calling convention to respect __regcall ABI v.4") LANGOPT(MatrixTypes, 1, 0, NotCompatible, "Enable or disable the builtin matrix type") +VALUE_LANGOPT(MaxMatrixDimension, 32, (1 << 20) - 1, NotCompatible, "maximum allowed matrix dimension") LANGOPT(CXXAssumptions, 1, 1, NotCompatible, "Enable or disable codegen and compile-time checks for C++23's [[assume]] attribute") diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index e403b3edde5d7..32c8f6209a693 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -4712,8 +4712,8 @@ QualType ASTContext::getConstantMatrixType(QualType ElementTy, unsigned NumRows, assert(MatrixType::isValidElementType(ElementTy) && "need a valid element type"); - assert(ConstantMatrixType::isDimensionValid(NumRows) && - ConstantMatrixType::isDimensionValid(NumColumns) && + assert(NumRows > 0 && NumRows <= LangOpts.MaxMatrixDimension && + NumColumns > 0 && NumColumns <= LangOpts.MaxMatrixDimension && "need valid matrix dimensions"); void *InsertPos = nullptr; if (ConstantMatrixType *MTP = MatrixTypes.FindNodeOrInsertPos(ID, InsertPos)) diff --git a/clang/lib/Basic/LangOptions.cpp b/clang/lib/Basic/LangOptions.cpp index 641a3dba0e67a..19b557603d135 100644 --- a/clang/lib/Basic/LangOptions.cpp +++ b/clang/lib/Basic/LangOptions.cpp @@ -132,8 +132,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(); diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp index e118dda4780e2..81c836fe60452 100644 --- a/clang/lib/Sema/HLSLExternalSemaSource.cpp +++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp @@ -159,7 +159,8 @@ void HLSLExternalSemaSource::defineHLSLMatrixAlias() { SourceLocation(), ColsParam)); TemplateParams.emplace_back(ColsParam); - const unsigned MaxMatDim = 4; + const unsigned MaxMatDim = SemaPtr->getLangOpts().MaxMatrixDimension; + ; auto *MaxRow = IntegerLiteral::Create( AST, llvm::APInt(AST.getIntWidth(AST.IntTy), MaxMatDim), AST.IntTy, SourceLocation()); diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 4f409ca0f414d..652527a88b160 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -16239,9 +16239,9 @@ getAndVerifyMatrixDimension(Expr *Expr, StringRef Name, Sema &S) { return {}; } uint64_t Dim = Value->getZExtValue(); - if (!ConstantMatrixType::isDimensionValid(Dim)) { + if (Dim == 0 || Dim > S.Context.getLangOpts().MaxMatrixDimension) { S.Diag(Expr->getBeginLoc(), diag::err_builtin_matrix_invalid_dimension) - << Name << ConstantMatrixType::getMaxElementsPerDimension(); + << Name << S.Context.getLangOpts().MaxMatrixDimension; return {}; } return Dim; diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 638904d6d0fb7..f740b6db66c2e 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -2517,12 +2517,12 @@ QualType Sema::BuildMatrixType(QualType ElementTy, Expr *NumRows, Expr *NumCols, Diag(AttrLoc, diag::err_attribute_zero_size) << "matrix" << ColRange; return QualType(); } - if (!ConstantMatrixType::isDimensionValid(MatrixRows)) { + if (MatrixRows > Context.getLangOpts().MaxMatrixDimension) { Diag(AttrLoc, diag::err_attribute_size_too_large) << RowRange << "matrix row"; return QualType(); } - if (!ConstantMatrixType::isDimensionValid(MatrixColumns)) { + if (MatrixColumns > Context.getLangOpts().MaxMatrixDimension) { Diag(AttrLoc, diag::err_attribute_size_too_large) << ColRange << "matrix column"; return QualType(); >From 17751d01beae161ca507e6efcdd97d941398bce0 Mon Sep 17 00:00:00 2001 From: Farzon Lotfi <[email protected]> Date: Tue, 14 Oct 2025 12:23:41 -0400 Subject: [PATCH 2/5] add unit test --- .../Frontend/CompilerInvocationTest.cpp | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp index 75390aa42d00e..1332422688fe6 100644 --- a/clang/unittests/Frontend/CompilerInvocationTest.cpp +++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp @@ -732,6 +732,26 @@ TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagPresent) { ASSERT_THAT(GeneratedArgs, Contains(StrEq("-sycl-std=2017"))); } +TEST_F(CommandLineTest, ConditionalParsingIfHLSLFlagPresent) { + const char *Args[] = {"-xhlsl"}; + + CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags); + + ASSERT_EQ(Invocation.getLangOpts().MaxMatrixDimension, 4u); + + Invocation.generateCC1CommandLine(GeneratedArgs, *this); +} + +TEST_F(CommandLineTest, ConditionalParsingIfHLSLFlagNotPresent) { + const char *Args[] = {""}; + + CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags); + + ASSERT_EQ(Invocation.getLangOpts().MaxMatrixDimension, 1048575u); + + Invocation.generateCC1CommandLine(GeneratedArgs, *this); +} + // Wide integer option. TEST_F(CommandLineTest, WideIntegerHighValue) { >From 1d1ff4536986ac5afd26424747512d87e8c86859 Mon Sep 17 00:00:00 2001 From: Farzon Lotfi <[email protected]> Date: Wed, 15 Oct 2025 11:12:37 -0400 Subject: [PATCH 3/5] update tests to confirm matrix_type can not be larger than 4 per dim --- clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl index 6f6b01bac829e..63d7acfcd647f 100644 --- a/clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl +++ b/clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl @@ -10,3 +10,9 @@ uint16_t4x4 mat2; 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}} + +using float8x4 = __attribute__((matrix_type(8,4))) float; +// expected-error@-1 {{matrix row size too large}} + +using float4x8 = __attribute__((matrix_type(4,8))) float; +// expected-error@-1 {{matrix column size too large}} >From 98a61b20f90b3a2909d72659b9703f94a28a30d6 Mon Sep 17 00:00:00 2001 From: Farzon Lotfi <[email protected]> Date: Wed, 15 Oct 2025 12:11:35 -0400 Subject: [PATCH 4/5] add more tests and cover the row and column size too large case with a new diagnostic --- clang/lib/Sema/SemaType.cpp | 6 ++++++ clang/test/SemaCXX/matrix-type.cpp | 1 + .../BuiltIns/matrix-basic_types-errors.hlsl | 17 +++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index f740b6db66c2e..7c1fb12a549e6 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -2517,6 +2517,12 @@ QualType Sema::BuildMatrixType(QualType ElementTy, Expr *NumRows, Expr *NumCols, Diag(AttrLoc, diag::err_attribute_zero_size) << "matrix" << ColRange; return QualType(); } + if (MatrixRows > Context.getLangOpts().MaxMatrixDimension && + MatrixColumns > Context.getLangOpts().MaxMatrixDimension) { + Diag(AttrLoc, diag::err_attribute_size_too_large) + << RowRange << ColRange << "matrix row and column"; + return QualType(); + } if (MatrixRows > Context.getLangOpts().MaxMatrixDimension) { Diag(AttrLoc, diag::err_attribute_size_too_large) << RowRange << "matrix row"; diff --git a/clang/test/SemaCXX/matrix-type.cpp b/clang/test/SemaCXX/matrix-type.cpp index 186d3b6b35208..0f9bff868adbe 100644 --- a/clang/test/SemaCXX/matrix-type.cpp +++ b/clang/test/SemaCXX/matrix-type.cpp @@ -14,6 +14,7 @@ void matrix_var_dimensions(int Rows, unsigned Columns, char C) { using matrix7_t = int __attribute__((matrix_type(1, 0))); // expected-error{{zero matrix size}} using matrix7_t = int __attribute__((matrix_type(char, 0))); // expected-error{{expected '(' for function-style cast or type construction}} using matrix8_t = int __attribute__((matrix_type(1048576, 1))); // expected-error{{matrix row size too large}} + using matrix8_t = int __attribute__((matrix_type(1048576, 1048576))); // expected-error{{matrix row and column size too large}} } struct S1 {}; diff --git a/clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl index 63d7acfcd647f..5ad1d6aefde38 100644 --- a/clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl +++ b/clang/test/SemaHLSL/BuiltIns/matrix-basic_types-errors.hlsl @@ -16,3 +16,20 @@ using float8x4 = __attribute__((matrix_type(8,4))) float; using float4x8 = __attribute__((matrix_type(4,8))) float; // expected-error@-1 {{matrix column size too large}} + +using float8x8 = __attribute__((matrix_type(8,8))) float; +// expected-error@-1 {{matrix row and column size too large}} + +using floatNeg1x4 = __attribute__((matrix_type(-1,4))) float; +// expected-error@-1 {{matrix row size too large}} +using float4xNeg1 = __attribute__((matrix_type(4,-1))) float; +// expected-error@-1 {{matrix column size too large}} +using floatNeg1xNeg1 = __attribute__((matrix_type(-1,-1))) float; +// expected-error@-1 {{matrix row and column size too large}} + +using float0x4 = __attribute__((matrix_type(0,4))) float; +// expected-error@-1 {{zero matrix size}} +using float4x0 = __attribute__((matrix_type(4,0))) float; +// expected-error@-1 {{zero matrix size}} +using float0x0 = __attribute__((matrix_type(0,0))) float; +// expected-error@-1 {{zero matrix size}} >From 74c9738d983248316cd78f628ab7486b827d34a8 Mon Sep 17 00:00:00 2001 From: Farzon Lotfi <[email protected]> Date: Wed, 15 Oct 2025 21:10:51 -0400 Subject: [PATCH 5/5] fix typo --- clang/lib/Sema/HLSLExternalSemaSource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp index 81c836fe60452..f28a037202da5 100644 --- a/clang/lib/Sema/HLSLExternalSemaSource.cpp +++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp @@ -160,7 +160,7 @@ void HLSLExternalSemaSource::defineHLSLMatrixAlias() { TemplateParams.emplace_back(ColsParam); const unsigned MaxMatDim = SemaPtr->getLangOpts().MaxMatrixDimension; - ; + auto *MaxRow = IntegerLiteral::Create( AST, llvm::APInt(AST.getIntWidth(AST.IntTy), MaxMatDim), AST.IntTy, SourceLocation()); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
