Bug 49739 - [Matrix] Support #pragma clang fp

[effective-light]

Summary

From https://bugs.llvm.org/show_bug.cgi?id=49739:

Currently, #pragma clang fp are ignored for matrix types.

For the code below, the contract fast-math flag should be added to the generated call to llvm.matrix.multiply and fadd

typedef float fx2x2_t __attribute__((matrix_type(2, 2)));

void foo(fx2x2_t &A, fx2x2_t &C, fx2x2_t &B) {
  #pragma clang fp contract(fast)
  C = A*B + C;
}

[fhahn]

Awesome, thanks for the patch! I'm also adding @mibintc who I think was working on adding support for the pragma & co.

Could you add a brief description to the patch and also reference the bug report on bugs.llvm.org?

clang/test/CodeGen/fp-matrix-pragma.c

1

I don't think we need --target=arm64-unknown-iphoneos or -g0?

23

could you also add a float test that has both contract(fast) reassociate(on)?

[effective-light]

Remove the unnecessary flags and add another test

[mibintc]

The diff appears to be 2 separate commits, so on first glance this is only patching the test files. Usually if I am working on a patch and have responded to comments, I compress the patch+updates into a single commit (git rebase -i) before creating a diff to upload to Phabricator.

Are there Matrix builtins that you wish to have decorated with the FMF? e.g. look at CGBuiltin.cpp

[effective-light]

Whoops

[kpn]

I don't know the matrix implementation so I can't swear this hits every place needed, but the uses of CGFPOptionsRAII in this patch look correct at least.

[effective-light]

Fix build

[effective-light]

Use the correct clang command.

[fhahn]

In D100834#2702550, @kpn wrote:

I don't know the matrix implementation so I can't swear this hits every place needed, but the uses of CGFPOptionsRAII in this patch look correct at least.

Other parts of the extension include __builtin_matrix_transpose, indexing into a matrix and casting, but I don't think the FMFs are needed there. One thing that would be good to also test would be the compound operators, (-=, +=, *=). @effective-light it would be great if you could add a test for those, then LGTM from my side. If you need someone to commit the change on your behalf, please let us know and share the name + email to use for the commit authorship.

[effective-light]

Add a test for compound operations

[effective-light]

In D100834#2704330, @fhahn wrote:

In D100834#2702550, @kpn wrote:

I don't know the matrix implementation so I can't swear this hits every place needed, but the uses of CGFPOptionsRAII in this patch look correct at least.

Other parts of the extension include __builtin_matrix_transpose, indexing into a matrix and casting, but I don't think the FMFs are needed there. One thing that would be good to also test would be the compound operators, (-=, +=, *=). @effective-light it would be great if you could add a test for those, then LGTM from my side. If you need someone to commit the change on your behalf, please let us know and share the name + email to use for the commit authorship.

Ya, you can commit it under name: Hamza Mahfooz, email: [e-mail], thanks.

[fhahn]

LGTM, thanks!

I'll commit the patch shortly.

[fhahn]

@effective-light just FYI, if you are interested in improving fast-math flags handling during matrix lowering on the LLVM side, there's https://bugs.llvm.org/show_bug.cgi?id=49738

clang/test/CodeGen/fp-matrix-pragma.c

-// RUN: %clang_cc1 -emit-llvm -S -fenable-matrix --target=arm64-unknown-iphoneos -g0 -mllvm -disable-llvm-optzns -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -S -fenable-matrix -mllvm -disable-llvm-optzns -o - | FileCheck %s
 
 typedef float fx2x2_t __attribute__((matrix_type(2, 2)));
 typedef int ix2x2_t __attribute__((matrix_type(2, 2)));
 
 fx2x2_t fp_matrix_contract(fx2x2_t a, fx2x2_t b, float c, float d) {
 // CHECK: call contract <4 x float> @llvm.matrix.multiply.v4f32.v4f32.v4f32
 // CHECK: fdiv contract <4 x float>
 // CHECK: fmul contract <4 x float>
 #pragma clang fp contract(fast)
   return (a * b / c) * d;
 }
 
 fx2x2_t fp_matrix_reassoc(fx2x2_t a, fx2x2_t b, fx2x2_t c) {
 // CHECK: fadd reassoc <4 x float>
 // CHECK: fsub reassoc <4 x float>
 #pragma clang fp reassociate(on)
   return a + b - c;
 }
 
+fx2x2_t fp_matrix_ops(fx2x2_t a, fx2x2_t b, fx2x2_t c) {
+// CHECK: call reassoc contract <4 x float> @llvm.matrix.multiply.v4f32.v4f32.v4f32
+// CHECK: fadd reassoc contract <4 x float>
+#pragma clang fp contract(fast) reassociate(on)
+  return a * b + c;
+}
+
 ix2x2_t int_matrix_ops(ix2x2_t a, ix2x2_t b, ix2x2_t c) {
 // CHECK: call <4 x i32> @llvm.matrix.multiply.v4i32.v4i32.v4i32
 // CHECK: add <4 x i32>
 #pragma clang fp contract(fast) reassociate(on)
   return a * b + c;
 }