`column_major` and `row_major` don't apply correctly to template-dependent types

[llvm-beanz]

This is caused by the same issue as #4583, so the required transformation is similar.

This test case should cover the interesting cases:

// RUN: %clang_cc1 -HV 2021 -fsyntax-only -ffreestanding -verify %s

template <typename T, int X, int Y>
struct Matrices {
  row_major matrix<T, X, Y> RowMajor;
  column_major matrix<T, X, Y> RowMajor;

  row_major T NotAMatrix;        // expected-error {{'row_major' can only be used with a matrix type}}
  column_major T AlsoNotAMatrix; // expected-error {{'column_major' can only be used with a matrix type}}
};

struct AlsoMatrices {
  row_major matrix<float, 4, 4> RowMajor;
  column_major matrix<float, 4, 4> RowMajor;

  row_major float NotAMatrix;        // expected-error {{'row_major' can only be used with a matrix type}}
  column_major float AlsoNotAMatrix; // expected-error {{'column_major' can only be used with a matrix type}}
};

void fn() {
  Matrices<float, 4, 4> ShouldWork;
}

[llvm-beanz]

I was hoping this was a relatively small task (similar to globallycoherent), but it really isn't.

There's a complicated mess here, with a lot of parts and a lot of the row_major/column_major base pieces don't work. For example, we allow the attributes on type annotations (i.e. typedef row_major float4x4 RowMajorf4x4), but we drop the annotations.

We also broke the source locations for the attributed type which causes the ASTDumper to crash.

The fix for the ASTDumper crash is:

diff --git a/tools/clang/lib/Sema/SemaType.cpp b/tools/clang/lib/Sema/SemaType.cpp
index 3a6c4ec9b..ad0dd60c6 100644
--- a/tools/clang/lib/Sema/SemaType.cpp
+++ b/tools/clang/lib/Sema/SemaType.cpp
@@ -4538,9 +4538,13 @@ static void fillAttributedTypeLoc(AttributedTypeLoc TL,
                                   const AttributeList *DeclAttrs = nullptr) {

   // HLSL changes begin
-  // Don't fill the location info for matrix orientation attributes
-  if (TL.getAttrKind() == AttributedType::attr_hlsl_row_major ||
-      TL.getAttrKind() == AttributedType::attr_hlsl_column_major)
+  // If there are no specified attributes we should skip filling attribute
+  // information for row_major and column_major attributes. This is required
+  // since the row_major and column_major attributes can be applied via
+  // #pragmas.
+  if (!(attrs || DeclAttrs) &&
+      (TL.getAttrKind() == AttributedType::attr_hlsl_row_major ||
+       TL.getAttrKind() == AttributedType::attr_hlsl_column_major))
     return;
   // HLSL changes end

All in all, since this isn't a straightforward fix, we should put this through team triaging to prioritize it against our other work.

[devshgraphicsprogramming]

can this be fixed with the new attributes being possible to be attached to typedefs? or does it only work on the vk_extension and vk_capability ?

[llvm-beanz]

The problem is attributes getting dropped during type canonicalization, that can't be helped by adding more attributes.

There are a lot of fundamental problems with how the matrix orientation attributes are designed, and we need to spend some time really thinking through the design to fix these correctly.

Part of the problem is that the matrix orientation attribute doesn't actually change the orientation of the matrix in thread-local memory, it just changes how it is read from or written to device or shared memory. We will be spending some time thinking through this design problem this year.