[Matrix] Lowering pass should also run at O0

[SjoerdMeijer]

Summary

As the Matrix intrinsic lowering pass is not running at -O0, code using the matrix extension results in a backend crash.

[SjoerdMeijer]

Fix typo in comment.

[fhahn]

There is already a patch to run a simple version of the lowering as part of the target pipelines D76858. I didn't land it yet, as I first wanted to come up with a lightweight system to figure out if the lowering pass actually needs to run on a function beforehand, to keep compile times low, if no matrix intrinsics are present. But I did not have time to wrap this up yet.

[SjoerdMeijer]

Ah okay, cheers, missed that. Time to abandon this.

[fhahn]

In D83570#2144734, @SjoerdMeijer wrote:

Ah okay, cheers, missed that. Time to abandon this.

I just committed a similar fix upstream, because there are some bots building the test-suite with -O0, to unblock them. The proper fix (run in lightweight lowering in the backend) will need a bit more time unfortunately.

[SjoerdMeijer]

In D83570#2155389, @fhahn wrote:

In D83570#2144734, @SjoerdMeijer wrote:

Ah okay, cheers, missed that. Time to abandon this.

I just committed a similar fix upstream, because there are some bots building the test-suite with -O0, to unblock them. The proper fix (run in lightweight lowering in the backend) will need a bit more time unfortunately.

Cheers.
And I will commit the test of this patch then as that still seems useful to me.

[SjoerdMeijer]

Okay, committing the test was an interesting exercise, that show there's a bot running the new pass manager which isn't happy with us:

http://lab.llvm.org:8011/builders/clang-x86_64-debian-new-pass-manager-fast/builds/11859/steps/test-check-all/logs/FAIL%3A%20Clang%3A%3Amatrix-lowering-opt-levels.c

I guess that means we have to sort out the passes for the NPM too @fhahn ?
Probably best if I revert my little test changes for now.

[fhahn]

In D83570#2155850, @SjoerdMeijer wrote:

Okay, committing the test was an interesting exercise, that show there's a bot running the new pass manager which isn't happy with us:

http://lab.llvm.org:8011/builders/clang-x86_64-debian-new-pass-manager-fast/builds/11859/steps/test-check-all/logs/FAIL%3A%20Clang%3A%3Amatrix-lowering-opt-levels.c

I guess that means we have to sort out the passes for the NPM too @fhahn ?

Oh yes, we need a NPM version of the pass as well.

[SjoerdMeijer]

I needed a few attempts :-( but have left a FIXME in the test case, and have disabled it by commented out the -O0 test in rG0160ad802e89.

Just checking again to avoid duplication: are you looking into enabling this with the NPM too? I wouldn't mind having a look.

[fhahn]

In D83570#2156035, @SjoerdMeijer wrote:

I needed a few attempts :-( but have left a FIXME in the test case, and have disabled it by commented out the -O0 test in rG0160ad802e89.

Just checking again to avoid duplication: are you looking into enabling this with the NPM too? I wouldn't mind having a look.

I probably won't have time to do so in the next few days, so if you would like to put up a patch, that would be great!

[SjoerdMeijer]

No problem, going to have a look at that.

clang/test/CodeGen/matrix-lowering.c

+// RUN: %clang -O0 -fenable-matrix -target aarch64-linux-eabi %s -S -emit-llvm -o - | FileCheck  %s
+// RUN: %clang -O1 -fenable-matrix -target aarch64-linux-eabi %s -S -emit-llvm -o - | FileCheck  %s
+// RUN: %clang -O2 -fenable-matrix -target aarch64-linux-eabi %s -S -emit-llvm -o - | FileCheck  %s
+// RUN: %clang -O3 -fenable-matrix -target aarch64-linux-eabi %s -S -emit-llvm -o - | FileCheck  %s
+// RUN: %clang -Ofast -fenable-matrix -target aarch64-linux-eabi %s -S -emit-llvm -o - | FileCheck  %s
+// RUN: %clang -Os -fenable-matrix -target aarch64-linux-eabi %s -S -emit-llvm -o - | FileCheck  %s
+// RUN: %clang -Oz -fenable-matrix -target aarch64-linux-eabi %s -S -emit-llvm -o - | FileCheck  %s
+
+// CHECK-NOT: @llvm.matrix.multiply
+
+typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+m4x4_t f(m4x4_t a, m4x4_t b, m4x4_t c) {
+//
+// CHECK-LAVEL: f(
+// CHECK-NOT:     @llvm.matrix.multiply
+// CHECK:       }
+//
+  return a + b * c;
+}
+

llvm/lib/Transforms/IPO/PassManagerBuilder.cpp

if (OptLevel == 0) {
     if (PrepareForLTO || PrepareForThinLTO) {
       MPM.add(createCanonicalizeAliasesPass());
       // Rename anon globals to be able to export them in the summary.
       // This has to be done after we add the extensions to the pass manager
       // as there could be passes (e.g. Adddress sanitizer) which introduce
       // new unnamed globals.
       MPM.add(createNameAnonGlobalPass());
     }
+    // Matrix intrinsics need to be lowered also at -O0, but don't run CSE as a
+    // clean-up after it as we do with OptLevel > 0.
+    MPM.add(createLowerMatrixIntrinsicsPass());
     return;
   }
 
   // Add LibraryInfo if we have some.
   if (LibraryInfo)
     MPM.add(new TargetLibraryInfoWrapperPass(*LibraryInfo));
 
   addInitialAliasAnalysisPasses(MPM);