This is an archive of the discontinued LLVM Phabricator instance.

[Matrix] Add draft specification for matrix support in Clang.
ClosedPublic

Authored by fhahn on Mar 23 2020, 7:38 AM.

Details

Summary

This patch documents the planned matrix support in Clang, based on the
draft specification discussed on cfe-dev in the 'Matrix Support in
Clang' thread.

Latest draft spec sent to cfe-dev: http://lists.llvm.org/pipermail/cfe-dev/2020-February/064742.html
Discussion thread January: http://lists.llvm.org/pipermail/cfe-dev/2020-January/064206.html
Discussion thread March: http://lists.llvm.org/pipermail/cfe-dev/2020-March/064834.html

Diff Detail

Event Timeline

fhahn created this revision.Mar 23 2020, 7:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2020, 7:38 AM
Herald added a subscriber: tschuett. · View Herald Transcript
fhahn updated this revision to Diff 252995.Mar 26 2020, 3:19 PM

Update according to comments on cfe-dev.

fhahn updated this revision to Diff 254505.Apr 2 2020, 6:43 AM

Update arithmetic conversion rules after recent discussion on cfe-dev.

fhahn updated this revision to Diff 254861.Apr 3 2020, 11:14 AM

Specify that standard conversion rules do not apply to assignments for matrix types.

fhahn updated this revision to Diff 255341.Apr 6 2020, 7:53 AM

Update standard conversion wording as suggested by @rjmccall

SjoerdMeijer added inline comments.
clang/docs/MatrixSupport.rst
254 ↗(On Diff #255341)

Hi Florian, just reading this for the first time, this is cool stuff, and just a drive-by comment:

this section, Example, looks like a good candidate to be moved to the "Matrixes" section in
clang/docs/LanguageExtensions.rst. This is Clang/LLVM specific, may not be that relevant for a language draft spec? But anyway, it may also be some of the user-facing examples missing in the language extension doc.

fhahn marked an inline comment as done.Apr 9 2020, 11:15 AM
fhahn added inline comments.
clang/docs/MatrixSupport.rst
254 ↗(On Diff #255341)

Ah yes, it seems like the LLVM IR lowering does not fit either here or in LanguageExtensions. It might be good to move the example to LanguageExtnesions though.

Scanned through the first bit.

clang/docs/LanguageExtensions.rst
500

This should include just a bit more detail about the extension. I would suggest:

Clang supports matrix types as an experimental extension. See :refmatrices` for more details.

clang/docs/MatrixSupport.rst
3 ↗(On Diff #255341)

This extension should be called something like "Matrices" or "Matrix Types". The "X Support" name makes it sound like it's a support layer for some external technology.

12 ↗(On Diff #255341)
  1. "Clang provides a C/C++ language extension that allows users to directly express fixed-size matrices as language values and perform arithmetic on them."
  1. This document *is* the specification, there's nothing to cross-reference it.
14 ↗(On Diff #255341)

"This feature is currently experimental, and both its design and its implementation are in flux."

30 ↗(On Diff #255341)

You can assume the existence of a hypothetical external language specification for GNU attribute syntax, so these starting paragraphs whittle down to:

Matrix types can be declared by adding the matrix_type attribute to the declaration of a typedef (or a C++ alias declaration). The underlying type of the typedef must be an unqualified integer or floating-point type. The attribute takes two arguments, both of which must be integer constant expressions that evaluate to a value greater than zero. The first specifies the number of rows, and the second specifies the number of columns. The underlying type of the typedef becomes a matrix type with the given dimensions and an element type of the former underlying type.

The paragraph about redeclarations is good.

48 ↗(On Diff #255341)

I would put this first before getting into the spelling. You can also put the stuff about implementation limits on dimensions in here.

55 ↗(On Diff #255341)

Do you actually want to guarantee this layout in the language specification? I would just say that a matrix includes storage for rows * columns elements but that the interior layout and overall size and alignment are implementation-defined.

57 ↗(On Diff #255341)

These are both important to include, but they're unrelated and shouldn't be in the same sentence.

64 ↗(On Diff #255341)

That doesn't belong in a language specification, but you could reasonably add a non-normative section at the end about the decisions that Clang currently makes for things like size, alignment, internal layout, and argument/result conventions.

106 ↗(On Diff #255341)

It would be better to say explicitly that "The index expressions shall..."

110 ↗(On Diff #255341)

I'd put all this like:

An expression of the form `E1 [E2] [E3], where E1 has matrix type cv M, is a matrix element access expression. Let T be the element type of M, and let R and C be the number of rows and columns in M respectively. The index expressions shall have integral or unscoped enumeration type and shall not be uses of the comma operator unless parenthesized. The first index expression shall evaluate to a non-negative value less than R, and the second index expression shall evaluate to a non-negative value less than C, or else the expression has undefined behavior. If E1 is a prvalue, the result is a prvalue with type T and is the value of the element at the given row and column in the matrix. Otherwise, the result is a glvalue with type cv T and with the same value category as E1` which refers to the element at the given row and column in the matrix.

118 ↗(On Diff #255341)

You should add a normative paragraph saying that a program is ill-formed if it insufficiently subscripts into a matrix.

fhahn updated this revision to Diff 257025.Apr 13 2020, 10:38 AM
fhahn marked 15 inline comments as done.

Address @rjmccall comments.

Scanned through the first bit.

Thanks a lot! I hope I managed to address the comments adequately.

clang/docs/MatrixSupport.rst
3 ↗(On Diff #255341)

I changed it to "Matrix Types"

fhahn added inline comments.Apr 13 2020, 10:39 AM
clang/docs/MatrixSupport.rst
30 ↗(On Diff #255341)

Thanks, that helps to simplify this section a lot.

48 ↗(On Diff #255341)

Sounds good! Moved.

57 ↗(On Diff #255341)

I moved the scalar type bit to the first sentence and move the part about the alignment to a separate sentence stating that the layout overall size and alignment are implementation defined.

64 ↗(On Diff #255341)

I've added a new Decisions for the Implementation in Clang section

110 ↗(On Diff #255341)

Updated, thanks.

118 ↗(On Diff #255341)

I added the following

Programs containing a single subscript expression into a matrix are ill-formed.

Reading through the rest of the spec.

clang/docs/LanguageExtensions.rst
500

If we're calling the extension "matrix types", that should be reflected in this section name and in the file name.

clang/docs/MatrixSupport.rst
28 ↗(On Diff #257025)

No need to italicize "element type" the second time.

The italics introduce a term, so consider italicizing "rows" and "columns" as well in the first sentence.

39 ↗(On Diff #257025)

Maybe break the TODOs here into their own sections, which would come much later.

70 ↗(On Diff #257025)

I don't think you need to list out the kinds of promotion and conversion here, and it doesn't make sense to define the "resulting type" this way when it's really a parameter. I'd just say:

A value of matrix type can be converted to another matrix type if the number of rows and columns are the size and the value's elements can be converted to the element type of the result type. The result is a matrix where each element is the converted corresponding element.

A value of non-matrix type can be converted to a matrix type if it can be converted to the element type of the matrix. The result is a matrix where all elements are the converted original value.

126 ↗(On Diff #257025)

I don't think this paragraph adds anything, and the restriction is kindof weird — it's just a restriction on when to consider applying these rules, rather than a restriction with absolute significance.

Also, "arithmetic type" includes unscoped enumeration types in both C and C++.

129 ↗(On Diff #257025)

Here I think you can say "where at least one of M1 or M2 is of matrix type and, for *, the other is of arithmetic type".

I think you'll need to separately describe the restrictions on +=, -=, and *=, but you should be able to say that the semantics are as if for the expansion.

176 ↗(On Diff #257025)

"builtin" should be capitalized here.

211 ↗(On Diff #257025)

This name sounds like it's loading a column, when I think you're saying that the memory has to be in column-major order.

I would call stride something like columnStride to make it clear that it's the stride between columns, as opposed to a stride between the elements within a column, which is also something that's theoretically interesting.

Should stride be an optional argument to make it easier to write the (I expect) common case where the matrix is dense?

fhahn updated this revision to Diff 257115.Apr 13 2020, 2:29 PM
fhahn marked 10 inline comments as done.

Address latest comments, thanks again!

fhahn added inline comments.Apr 13 2020, 2:35 PM
clang/docs/MatrixSupport.rst
39 ↗(On Diff #257025)

Done, I've moved the TODOs to a TODO section just after the builtins section.

70 ↗(On Diff #257025)

I've kept the first paragraph (including the exclusion of assignment) and replaced the second with your suggestion.

129 ↗(On Diff #257025)

I added the following at the end of the section

For the `+=, -= and *=` operators the semantics match their expanded variants.

211 ↗(On Diff #257025)

This name sounds like it's loading a column, when I think you're saying that the memory has to be in column-major order.

Yes that is correct. Maybe __builtin_matrix_columnwise_load would be slightly better?

Should stride be an optional argument to make it easier to write the (I expect) common case where the matrix is dense?

Yes that would be very convenient, especially now that casting between element wise pointers and matrixes is not allowed. I've added a sentence to the remarks for both the load and store builtins.

rjmccall added inline comments.Apr 13 2020, 3:24 PM
clang/docs/MatrixSupport.rst
211 ↗(On Diff #257025)

Yes that is correct. Maybe __builtin_matrix_columnwise_load would be slightly better?

The term of art is "column-major"; I don't think avoiding an "extra word" is a good enough reason to invent something else. __builtin_matrix_column_major_load sounds fine to me.

clang/docs/MatrixTypes.rst
79

You should standardize on one term and then be clear what you mean by it. Here you're saying "integer or floating point type", but elsewhere you use "arithmetic type". Unfortunately, the standard terms mean somewhat different things in different standards: "integer" includes enums in C but not in C++, "arithmetic" doesn't include complex types in C++ (although it does by extension in Clang), etc. I think for operands you probably want arithmetic types in the real domain (which in Clang is isRealType()). However, you'll want to use a narrower term for the restriction on element types because Clang does support fixed-point types, but you probably don't want to support matrices of them quite yet (and you may not want to allow matrices of bools, either).

Also, your description of the scalar conversions no longer promotes them to matrix type.

123

They also have to have the same element types, right? So they have to be the same types?

138

Same point about element types.

141

The easier way to put this now is that it's a matrix type whose element type is the common element type, but with the number of rows of M1 and the number of columns of M2.

152

inner is not defined.

164

This is about rounding, not rounding "errors".

The definition of matrix multiply you've written it above would actually permit an FMA under C's default rules.

More broadly, I think you need to define how the FP contraction and environment rules affect matrix arithmetic expressions. If FP contraction is enabled, can S * M1 + M2 perform elementwise FMAs?

fhahn updated this revision to Diff 257253.Apr 14 2020, 2:59 AM
fhahn marked 8 inline comments as done.

Address latest comments, thanks!

fhahn added inline comments.Apr 14 2020, 3:00 AM
clang/docs/MatrixTypes.rst
79

You should standardize on one term and then be clear what you mean by it. Here you're saying "integer or floating point type", but elsewhere you use "arithmetic type". Unfortunately, the standard terms mean somewhat different things in different standards: "integer" includes enums in C but not in C++, "arithmetic" doesn't include complex types in C++ (although it does by extension in Clang), etc. I think for operands you probably want arithmetic types in the real domain (which in Clang is isRealType()). However, you'll want to use a narrower term for the restriction on element types because Clang does support fixed-point types, but you probably don't want to support matrices of them quite yet (and you may not want to allow matrices of bools, either).

I've added the following to the Matrix Type section:
A *matrix element type* must be a real type (as in C99 6.2.5p17) excluding enumeration types or an implementation-defined half-precision floating point type, otherwise the program is ill-formed.

Other places are updated to use a valid matrix element type instead.

I think we explicitly want to allow half-precision types (like __fp16 and Float16 in Clang). I think by referring to real type as in the C99 spec, we naturally exclude Clang's fixed-point types and bool, right?

Also, your description of the scalar conversions no longer promotes them to matrix type.

Right, I think we can just refer to the standard conversion rules here, as in

If one operand is of matrix type and the other operand is of a valid matrix element type, convert the non-matrix type operand to the matrix type according to the standard conversion rules.

123

Yes, for 2 operands of a matrix type, they should be the same types now. Changed to M1 and M2 shall be of the same matrix type.

138

Added

The element types of `M1 and M2` shall be the same type

141

Replaced with

The resulting type, MTy, is a matrix type with the common element type, the number of rows of M1 and the number of columns of M2.

152

Should be something like and inner is the number of columns of M1``

164

This is about rounding, not rounding "errors".

Fixed.

The definition of matrix multiply you've written it above would actually permit an FMA under C's default rules.

The goal of the wording it to match the existing behavior for the expanded version for compatibility reasons.I think Clang currently does not emit FMAs without contraction explicitly enabled, but GCC emits FMAs without contraction explicitly enabled for something like A * B + C. I think the current wording allows for both, following the reasoning for both Clang's and GCC's current behavior for the single element case.

More broadly, I think you need to define how the FP contraction and environment rules affect matrix arithmetic expressions. If FP contraction is enabled, can S * M1 + M2 perform elementwise FMAs?

FP contraction and environment rules should match the corresponding expansions. So with fp-contraction enabled, S * M1 + M2. I re-worded the paragraph and hopefully it is clearer now:

With respect to floating-point contraction, rounding and environment rules, operations on matrix types match the behavior of the elementwise operations in the corresponding expansions provided above.

I've moved the part of the clang option to the Decision for the Implementation in Clang section.

fhahn updated this revision to Diff 257259.Apr 14 2020, 3:11 AM
fhahn marked an inline comment as done.

Rename builtin_matrix_columnwise_{load,store} => builtin_matrix_column_major_{load,store}

SjoerdMeijer added inline comments.Apr 14 2020, 4:23 AM
clang/docs/MatrixTypes.rst
13

Would it be good to set expectations here or in the section below: define that we're talking about 2-dimensional m × n matrices?

26

typo: ype -> type

28

above you're using *element type* and here *matrix element type*. Since hopefully we're talking about the same things, "matrix *element type*" would be more consistent.

But this is just a nit, my main question is about the types:
why not e.g. define this to be the C11 types, that include _FloatN types, so that we can include N=16? Or is this intentionally omitted? I haven't even checked if this is supported in the architecture extension, but might make sense? And also, an element type cannot be an integer type?

fhahn updated this revision to Diff 257304.Apr 14 2020, 6:18 AM

Fix typo, remove a 2 places where underlying element type was used, move C portion of the example to LanguageExtensions.rst, drop the rest of the example.
:

fhahn marked 3 inline comments as done.Apr 14 2020, 6:29 AM
fhahn added inline comments.
clang/docs/MatrixTypes.rst
13

I've changed it to fixed-size 2-dimensional matrices. I think the type definition below should be already clear enough about being 2 dimensional.

28

above you're using *element type* and here *matrix element type*. Since hopefully we're talking about the same things, "matrix *element type*" would be more consistent.

Yes it is referring to the same thing. I had a look at most uses, and in most cases element type is used to refer to the element type of a given matrix type. In that context it seems a bit verbose to use matrix element type, although I am more than happy to change that if it helps with clarifying things.

I intentionally used matrix element type in Arithmetic Conversions, because there it is standing on its own and refers exactly to the set of types defined as valid matrix element types here.

why not e.g. define this to be the C11 types, that include _FloatN types, so that we can include N=16? Or is this intentionally omitted? I haven't even checked if this is supported in the architecture extension, but might make sense?

I couldn't find any reference to _FloatN types in the C11 draft version I checked. Do you by any chance have a reference to the _FloatN types?

And also, an element type cannot be an integer type?

The current definition should include it (real types include integer and real floating point types according to C99 6.2.5p17). I don't think there is any reason to exclude them I think.

fhahn updated this revision to Diff 257309.Apr 14 2020, 6:40 AM

Drop another instance of underlying element type.

SjoerdMeijer added inline comments.Apr 14 2020, 6:50 AM
clang/docs/MatrixTypes.rst
28
why not e.g. define this to be the C11 types, that include _FloatN types, so that we can include N=16? Or is this intentionally omitted? I haven't even checked if this is supported in the architecture extension, but might make sense?

I couldn't find any reference to _FloatN types in the C11 draft version I checked. Do you by any chance have a reference to the _FloatN types?

Sorry, I was a bit imprecise here, it's an extension of C11: ISO/IEC TS 18661-3:2015.
My thinking was it would be cool to support the "proper" half-precision type. I thought about this, because of "or an implementation-defined half-precision" mentioned just below here, of which probably __fp16 is an example. If you refer to the C99 types, you probably don't even need to mention this (although it won't do any harm)?

And also, an element type cannot be an integer type?

The current definition should include it (real types include integer and real floating point types according to C99 6.2.5p17). I don't think there is any reason to exclude them I think.

Ok, cheers, wrote this from memory (forgot this), and didn't check the standard.

fhahn marked an inline comment as done.Apr 14 2020, 11:16 AM
fhahn added inline comments.
clang/docs/MatrixTypes.rst
28

Sorry, I was a bit imprecise here, it's an extension of C11: ISO/IEC TS 18661-3:2015.
My thinking was it would be cool to support the "proper" half-precision type. I thought about this, because of "or an implementation-defined half-precision" mentioned just below here, of which probably __fp16 is an example. If you refer to the C99 types, you probably don't even need to mention this (although it won't do any harm)?

I am not sure what the exact wording should be, but the intention is to include both __fp16 and _Float16. I was hoping that would be covered as is, but I would be happy to clarify (unfortunately it is not entirely clear to me how to best word this)

rjmccall added inline comments.Apr 14 2020, 12:31 PM
clang/docs/MatrixTypes.rst
79

I think we explicitly want to allow half-precision types (like __fp16 and Float16 in Clang). I think by referring to real type as in the C99 spec, we naturally exclude Clang's fixed-point types and bool, right?

C says:

The integer and real floating types are collectively called *real types*.

The type char, the signed and unsigned integer types, and the enumerated types are collectively called *integer types*.

The standard and extended unsigned integer types are collectively called *unsigned integer types*.

The type _Bool and the unsigned integer types that correspond to the standard signed integer types are the *standard unsigned integer types*.

Embedded C (TR 18037) says:

Clause 6.2.5 - Types, paragraph 17: change last sentence as follows.

Integer, fixed-point and real floating types are collectively called *real types*.

So you'll have to explicitly exclude enumerated types, _Bool, and the fixed-point types.

SjoerdMeijer added inline comments.Apr 14 2020, 12:39 PM
clang/docs/MatrixTypes.rst
28

Ah, okay, I got it. How about a simple enumeration, e.g.:

A *matrix element type* must be a C99 real type, excluding enumeration types, the C11 ISO/IEC TS 18661 _Float16 type, the ARM ACLE __fp16 type, or an implementation-defined half-precision floating point type, otherwise the program is ill-formed.
30

Now I am wondering if this requires some explanations on binary operations for these implemenation-defined types? For example, for __fp16 and matrices with this __fp16 element type, I assume arithmetic is performed in at least the (single) floating-point precision. So I guess in section "Arithmetic Conversions" a rule needs to be added that the conversion of these implementation defined types need to performed?

fhahn updated this revision to Diff 257741.Apr 15 2020, 8:50 AM
fhahn marked 3 inline comments as done.

Update list of types excluded from real types, thanks!

clang/docs/MatrixTypes.rst
28

Given that there are a few different half-precision floating point types with various levels of support in different compilers, I would prefer not to explicitly list them at the moment, while making it clear that they can also be supported. AFAIK there's work in progress to add Bfloat support to clang and I think we also would want to support that type in the future.

30

I don't think we need to specifically discuss the implementation defined types here, as the conversions and binary operator definitions are framed in terms of the existing rules for the element types used. I am potentially missing something, but with the current wording the conversions for __fp16 would use the conversion rules for that type and the binary operators would use the arithmetic rules for it.

79

Ah thanks, I missed TR 18037. Sorry about that! I've updated the wording as suggested.

rjmccall added inline comments.Apr 15 2020, 9:11 PM
clang/docs/MatrixTypes.rst
30

Yeah, for the scalar conversions / scalar operands, you should just say that the source has to be a real type and not otherwise restrict it. All of those types should already be convertible to any matrix element type.

fhahn updated this revision to Diff 258179.Apr 16 2020, 2:44 PM

Update wording to allow any real type for scalar -> matrix conversion and scalar,matrix binary ops.

fhahn marked an inline comment as done.Apr 16 2020, 2:45 PM
fhahn added inline comments.
clang/docs/MatrixTypes.rst
30

Thanks, I've updated the wording to ensure the scalar values are of a real type in the scalar -> matrix conversion and scalar, matrix binary operator contexts. I hope that is enough to clarify things.

rjmccall added inline comments.Apr 16 2020, 8:44 PM
clang/docs/LanguageExtensions.rst
511

This is kindof an unnecessarily unreadable example. I know you haven't decided on calling convention treatment yet, but maybe the leading example could be just a little ahead of the implementation and just take the matrices as arguments and then return the result.

clang/docs/MatrixTypes.rst
30

This would be clearer as something like:

Currently, the element type of a matrix is only permitted to be one of the following types:

  • an integer type (as in C2x 6.2.5p19), but excluding enumerated types and _Bool
  • a standard floating type (as in C2x 6.2.5p10)
  • a half-precision floating point type, if one is supported on the target

Other types may be supported in the future.

Although I don't know if you actually want to unconditionally support long double; you might just want to say "the standard floating types float and double".

63

s/size/same/

67

"A value of any real type (as in C2x 6.2.5p17) can be converted..."

168

The expansions have a lot of statement boundaries that contraction wouldn't be allowed across. I'd suggest saying something like:

Operations on floating-point matrices have the same rounding and floating-point environment behavior as ordinary floating-point operations in the expression's context. For the purposes of floating-point contraction, all calculations done as part of a matrix operation are considered intermediate operations, and their results need not be rounded to the format of the element type until the final result in the containing expression. This is subject to the normal restrictions on contraction, such as #pragma STDC FP_CONTRACT.

217

"omitted".

I would expect these operands to have type either size_t or ptrdiff_t. Of course it only really matters for columnStride.

fhahn updated this revision to Diff 258269.Apr 17 2020, 2:39 AM
fhahn marked 9 inline comments as done.

Update wordings as suggested, thanks!

clang/docs/LanguageExtensions.rst
511

I wasn't sure if that would be fine, but it indeed makes things much more readable. Updated.

clang/docs/MatrixTypes.rst
30

That's much better, thanks! I've also applied your suggestion to exclude long double for now.

168

Updated, thanks!

217

"omitted".

Done, thanks!

I would expect these operands to have type either size_t or ptrdiff_t. Of course it only really matters for columnStride.

Yes, I update them to size_t. This should give the implementations the most freedom with respect to choosing the implementation defined limits of rows/columns. size_t also makes the most sense for the stride I think, as it is required to be >= the number of rows in the matrix.

fhahn added a comment.Apr 24 2020, 6:24 AM

ping.

@rjmccall & @SjoerdMeijer thanks for all the comments. I hope they are no addressed adequately.

LGTM with one very minor fix.

clang/docs/LanguageExtensions.rst
511

Extra space after the +.

fhahn added a comment.Apr 24 2020, 2:33 PM

Thanks, I plan to submit this on Monday and then make sure the patches on the clang side align with the draft.

fhahn accepted this revision.Apr 27 2020, 9:59 AM

Mark as accepted to make Phabricator/arc happy

This revision is now accepted and ready to land.Apr 27 2020, 9:59 AM
This revision was automatically updated to reflect the committed changes.