[Docs] Define matrix initialisation in MatrixTypes documentation

[SaurabhJha]

Summary

As part of https://bugs.llvm.org/show_bug.cgi?id=46251, this patch adds definition of matrix initialisation. I am not very familiar with this part of C++
so please let me know if I am wrong here, which I suspect I am.

I have been trying to follow the reference documentation https://en.cppreference.com/w/cpp/language/initialization and the commented out test in the
bugzilla ticket https://github.com/llvm/llvm-project/blob/3323a628ec821b8b75d3b60bf1510931f97d3883/clang/test/CodeGenCXX/matrix-type-builtins.cpp#L78

[SaurabhJha]

Ping :)

[fhahn]

clang/docs/MatrixTypes.rst

271

is there a reason we need to allow 'empty' initialisation? What does it mean?

[SaurabhJha]

clang/docs/MatrixTypes.rst

271

By empty, I meant this {} initialisation which was in the test case. I got the term wrong but seems like we do want to support {} initialisation, right?

[SaurabhJha]

clang/docs/MatrixTypes.rst

271

I meant this test case https://github.com/llvm/llvm-project/blob/3323a628ec821b8b75d3b60bf1510931f97d3883/clang/test/CodeGenCXX/matrix-type-builtins.cpp#L78

[SaurabhJha]

Updated docs to address comments

[rjmccall]

clang/docs/MatrixTypes.rst

279

This is contradicted by your first example. I think you want to say something like

A value of matrix type M can be initialized with an initializer list:

(examples)

If the initializer list is empty, all elements of the matrix are zero-initialized. Otherwise, the initializer list must consist of M::rows initializer lists, each of which must consist of M::columns expressions, each of which is used to initialize the corresponding element of the matrix (that is, m[i][j] is initialized by the jth expression of the ith initializer list in the initializer). Element designators are not allowed.

That's assuming you want to allow {}, but I think that's probably a good idea. At the very least, you already have to define how objects of static storage duration are zero-initialized, and having a way to do that explicitly always seems wise to me.

[SaurabhJha]

clang/docs/MatrixTypes.rst

279

That sounds great @rjmccall, thanks for your suggestions.

I think I got confused between whether to allow {} but as you suggested, I think we should.

[SaurabhJha]

Address second round of comments

[fhahn]

Thank you very much for working on this! Are you planning on implementing the new specification as well? It would probably be good to land the update to the spec in close succession to the implementation, to avoid confusing users.

clang/docs/MatrixTypes.rst

279

That looks good, thanks!

Another thing to consider is whether we would like to provide a convenient syntax to initialise all elements with an user-specified value, i.e. should we allow something like m = {99};, which would broadcast 99 to all elements of the matrix?

[SaurabhJha]

In D106005#2895716, @fhahn wrote:

Thank you very much for working on this! Are you planning on implementing the new specification as well? It would probably be good to land the update to the spec in close succession to the implementation, to avoid confusing users.

Yes, that's my plan. Once this is in, I will start working on the implementation right away.

clang/docs/MatrixTypes.rst

279

That sounds good. Do we have a way to infer the number of rows and columns for a matrix object m so that we could broadcast correctly? I will amend the docs later today.

[SaurabhJha]

Add documentation for matrix broadcast initialization

[fhahn]

In D106005#2896080, @SaurabhJha wrote:

In D106005#2895716, @fhahn wrote:

Thank you very much for working on this! Are you planning on implementing the new specification as well? It would probably be good to land the update to the spec in close succession to the implementation, to avoid confusing users.

Yes, that's my plan. Once this is in, I will start working on the implementation right away.

Ok cool! I think the latest version looks good (modulo making sure the new lines are limited to 80 chars per line). @rjmccall can you think of any scenarios where defining initializers with one expression and broadcasting them might cause issues?

With respect to ordering the patches, I think it would be good to put up a patch implementing the newly added parts, commit it and then land the patch that adds it to the docs. WDYT?

[SaurabhJha]

In D106005#2904424, @fhahn wrote:

In D106005#2896080, @SaurabhJha wrote:

In D106005#2895716, @fhahn wrote:

Thank you very much for working on this! Are you planning on implementing the new specification as well? It would probably be good to land the update to the spec in close succession to the implementation, to avoid confusing users.

Yes, that's my plan. Once this is in, I will start working on the implementation right away.

Ok cool! I think the latest version looks good (modulo making sure the new lines are limited to 80 chars per line). @rjmccall can you think of any scenarios where defining initializers with one expression and broadcasting them might cause issues?

With respect to ordering the patches, I think it would be good to put up a patch implementing the newly added parts, commit it and then land the patch that adds it to the docs. WDYT?

Yeah, sounds good. I will create a patch for implementing initialisation.

[fhahn]

In D106005#2904865, @SaurabhJha wrote:

In D106005#2904424, @fhahn wrote:

In D106005#2896080, @SaurabhJha wrote:

In D106005#2895716, @fhahn wrote:

Thank you very much for working on this! Are you planning on implementing the new specification as well? It would probably be good to land the update to the spec in close succession to the implementation, to avoid confusing users.

Yes, that's my plan. Once this is in, I will start working on the implementation right away.

Ok cool! I think the latest version looks good (modulo making sure the new lines are limited to 80 chars per line). @rjmccall can you think of any scenarios where defining initializers with one expression and broadcasting them might cause issues?

With respect to ordering the patches, I think it would be good to put up a patch implementing the newly added parts, commit it and then land the patch that adds it to the docs. WDYT?

Yeah, sounds good. I will create a patch for implementing initialisation.

@SaurabhJha did you ever get a chance to create a patch?

[SaurabhJha]

@SaurabhJha did you ever get a chance to create a patch?

Hey @Florian, sorry I dropped the ball here. I don't think I would be able to spend time on this so abandoning this revision.

clang/docs/MatrixTypes.rst

.. code-block:: c++
 
   for (size_t C = 0; C < columns in M; ++C) {
     for (size_t R = 0; R < rows in M; ++K)
       ptr[R] = matrix[R][C];
     ptr += columnStride
   }
 
 
+Initialization Syntax
+---------------------
+An empty value of a matrix type M can be initialised using this syntax:
+
+.. code-block:: c++
+
+  constexpr M m = {};
+
+A non-empty value of a matrix type M can be initialised using the nested initialisation syntax:
+
+.. code-block: c++
+  constexpr M m = {{a, b, c}, {d, e, f}}
+
+where the number of constituent arrays must equal the number rows in the matrix type M and the number of elements
+in each constituent array must equal the number of columns in the matrix type.
+
 TODOs
 -----
 
 TODO: Does it make sense to allow M::element_type, M::rows, and M::columns
 where M is a matrix type? We don’t support this anywhere else, but it’s
 convenient. The alternative is using template deduction to extract this
 information. Also add spelling for C.
 
-Future Work: Initialization syntax.
-
-
 Decisions for the Implementation in Clang
 =========================================
 
 This section details decisions taken for the implementation in Clang and is not
 part of the draft specification.
 
 The elements of a  value of a matrix type are laid out in column-major order
 without padding.
 
 We propose to provide a Clang option to override this behavior and allow
 contraction of those operations (e.g. *-ffp-contract=matrix*).
 
 TODO: Specify how matrix values are passed to functions.