Page MenuHomePhabricator

[Matrix] Add support for matrix-by-scalar division.
ClosedPublic

Authored by fhahn on Mar 3 2021, 7:05 AM.

Details

Summary

This patch extends the matrix spec to allow matrix-by-scalar division.

Originally support for / was left out to avoid ambiguity for the
matrix-matrix version of /, which could either be elementwise or
specified as matrix multiplication M1 * (1/M2).

For the matrix-scalar version, no ambiguity exists; * is also
an elementwise operation in that case. Matrix-by-scalar division
is commonly supported by systems including Matlab, Mathematica
or NumPy.

Diff Detail

Event Timeline

fhahn created this revision.Mar 3 2021, 7:05 AM
fhahn requested review of this revision.Mar 3 2021, 7:05 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 3 2021, 7:05 AM
clang/test/CodeGen/matrix-type-operators.c
717–827

Shouldn't a test for half floating point be added here as well?

fhahn updated this revision to Diff 329596.Mar 10 2021, 3:22 AM

rebase & added a matrix_types_scalar_division feature, which allows users to check if the current version of clang is new enough to support it.

fhahn added inline comments.Mar 10 2021, 3:25 AM
clang/test/CodeGen/matrix-type-operators.c
717–827

Thanks for taking a look! I don't think we have any tests for half floating points at the moment. The tests do not cover each possible combination of types I think, because they re-use the existing code-gen for the stuff that's element type specific (except deciding whether to use floating point or integer ops).

I think it would probably be good to have some FP16 tests though, but that should be a separate change IMO.

Gerolf added a subscriber: Gerolf.Mar 10 2021, 11:25 AM

Nice! This is a straightforward extension with minor code clean-up.

clang/test/CodeGen/matrix-type-operators.c
717–827

Thanks for the reply. Ok, that makes sense, I was under the false impression FP16 was already covered.

rjmccall added inline comments.Mar 10 2021, 1:31 PM
clang/docs/MatrixTypes.rst
121–135

You need to rework this paragraph, something like:

For the expression `M1 BIN_OP M2` where

  • `BIN_OP is one of + or -, one of M1 and M2` is of matrix type, and the other is of matrix type or real type; or
  • `BIN_OP is *, one of M1 and M2` is of matrix type, and the other is of a real type; or
  • `BIN_OP is /, M1 is of matrix type, and M2` is of a real type:
122

You don't need to be so formal in this paragraph, because you're about to get into a more formal treatment. I'd just say something like:

Given two matrixes, the `+ and - operators perform element-wise addition and subtraction, while the * operator performs matrix multiplication. +, -, *, and /` can also be used with a matrix and a scalar value, applying the operation to each element of the matrix.

Earlier versions of this extension did not support division by a scalar. You can test for the availability of this feature with `__has_extension(matrix_types_scalar_division)`.

clang/include/clang/Basic/LangOptions.def
399 ↗(On Diff #329596)

Why is this a language option?

fhahn updated this revision to Diff 330000.Mar 11 2021, 9:40 AM

Thank you very much John! I applied your suggestions to the wording and removed the unneeded language feature.

This revision is now accepted and ready to land.Mar 11 2021, 1:44 PM
This revision was landed with ongoing or failed builds.Mar 11 2021, 2:21 PM
This revision was automatically updated to reflect the committed changes.