Page MenuHomePhabricator

[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

There are a very large number of changes, so older changes are hidden. Show Older Changes
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
80

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.

124

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

139

Same point about element types.

142

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.

153

inner is not defined.

165

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
80

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.

124

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.

139

Added

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

142

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.

153

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

165

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
80

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.

80

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.