[Matrix] Also run LowerMatrixIntrinsics for -O0.

[fhahn]

Summary

The lowering of the llvm.matrix intrinsics happens exclusively in the
LowerMatrixIntrinsics pass and the backends do not know about those
intrinsics. That means we also have to run LowerMatrixIntrinsics for
-O0. Otherwise Clang will crash in the backend for code with llvm.matrix
intrinsics.

Fixes PR45227.

[fhahn]

Ping.

Updated to add a test.

[nicolasvasilache]

I don't imagine this is particularly intrusive, LGTM.

[efriedma]

Normally most "must-run" passes are part of the codegen pipeline (TargetPassConfig). It's convenient if the backend can handle arbitrary bitcode files without having to run some arbitrary set of "opt" passes first.

[fhahn]

In D76327#1942729, @efriedma wrote:

Normally most "must-run" passes are part of the codegen pipeline (TargetPassConfig). It's convenient if the backend can handle arbitrary bitcode files without having to run some arbitrary set of "opt" passes first.

Thanks for pointing me to TargetPassConfig! Ideally the lowering pass would run unconditionally, but should be a no-op for functions that do not contain any matrix intrinsics. I think a good option would be to require the frontends to add an attribute ('may-contain-matrix-intrinsics') to functions that may contain matrix intrinsics. The LowerMatrixIntrinsics pass is run unconditionally, but bails out early for functions without the attribute. I've put up a patch D76857

This also should allow to add it unconditionally to TargetPassConfig and ensures that we do not run it unnecessarily in the backend again, if the intrinsics were already lowered in the middle-end: D76858

For running as part of the backend pipeline, I propose adding a simplified/minimal lowering pass, that only requires TTI, as we are planning on making the lowering pass more involved and running it in the backend should be last-resort kind of thing.

Does that approach seem reasonable?

[efriedma]

The attribute seems like overkill as a performance optimization. You can get 99% of the way there by just checking if if the module declares any matrix intrinsics.

Otherwise seems fine.

[fhahn]

In D76327#1944121, @efriedma wrote:

The attribute seems like overkill as a performance optimization. You can get 99% of the way there by just checking if if the module declares any matrix intrinsics.

Hmm, is there an easy way to check that without checking all the declarations in a module? Or would we have to check all declarations for all functions in a module?

[efriedma]

I assume the matrix intrinsics overloaded? In that case, you'd have to iterate over the module, yes.

I'm tempted to say we should add a new mechanism to the backend for passes that only care about calls to a specific set of intrinsics, so we don't have to reinvent this repeatedly.

[fhahn]

In D76327#1944354, @efriedma wrote:

I assume the matrix intrinsics overloaded? In that case, you'd have to iterate over the module, yes.

Unfortunately yes :(

I'm tempted to say we should add a new mechanism to the backend for passes that only care about calls to a specific set of intrinsics, so we don't have to reinvent this repeatedly.

We would also need it in the middle-end for the matrix lowering (and potentially others as well).

Do you have anything in particular in mind? For intrinsics that get solely emitted by the frontend, I guess it would be possible check if the module contains any such instructions and store it and have something like Module::containsMatrixIntrinsics(). Not sure if that's acceptable though.

[efriedma]

We could make the Module automatically compute a map from intrinsics to overloads of that intrinsic in the module, I guess. Probably not too expensive to maintain.

[fhahn]

In D76327#1944749, @efriedma wrote:

We could make the Module automatically compute a map from intrinsics to overloads of that intrinsic in the module, I guess. Probably not too expensive to maintain.

Sounds good. I'll prepare a patch.

[fhahn]

In D76327#1944749, @efriedma wrote:

We could make the Module automatically compute a map from intrinsics to overloads of that intrinsic in the module, I guess. Probably not too expensive to maintain.

I've put up D76941. Is that along the lines you suggested?

[fhahn]

Relevant parts landed in cbe0e539e79e

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());
     }
+
+    // Lower the matrix intrinsics to regular LLVM instructions, even for O0.
+    if (EnableMatrix)
+      MPM.add(createLowerMatrixIntrinsicsPass());
     return;
   }
 
   // Add LibraryInfo if we have some.
   if (LibraryInfo)
     MPM.add(new TargetLibraryInfoWrapperPass(*LibraryInfo));
 
   addInitialAliasAnalysisPasses(MPM);

llvm/test/Other/opt-O0-pipeline-enable-matrix.ll

+; RUN: opt -mtriple=x86_64-- -O0 -enable-matrix -debug-pass=Structure < %s -o /dev/null 2>&1 | FileCheck %s --check-prefixes=CHECK,%llvmcheckext
+
+; REQUIRES: asserts
+
+; CHECK-LABEL: Pass Arguments:
+; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT:   FunctionPass Manager
+; CHECK-NEXT:     Module Verifier
+; CHECK-EXT:     Good Bye World Pass
+; CHECK-NEXT:     Instrument function entry/exit with calls to e.g. mcount() (pre inlining)
+; CHECK-NEXT: Pass Arguments:
+; CHECK-NEXT: Target Library Information
+; CHECK-NEXT: Target Transform Information
+;             Target Pass Configuration
+; CHECK:      Assumption Cache Tracker
+; CHECK-NEXT: Profile summary info
+; CHECK-NEXT:   ModulePass Manager
+; CHECK-NEXT:     Force set function attributes
+; CHECK-NEXT:     CallGraph Construction
+; CHECK-NEXT:     Call Graph SCC Pass Manager
+; CHECK-NEXT:       Inliner for always_inline functions
+;                   A No-Op Barrier Pass
+; CHECK:            FunctionPass Manager
+; CHECK-NEXT:         Dominator Tree Construction
+; CHECK-NEXT:         Natural Loop Information
+; CHECK-NEXT:         Lazy Branch Probability Analysis
+; CHECK-NEXT:         Lazy Block Frequency Analysis
+; CHECK-NEXT:         Optimization Remark Emitter
+; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
+; CHECK-NEXT:         Function Alias Analysis Results
+; CHECK-NEXT:         Lower the matrix intrinsics
+; CHECK-NEXT:         Module Verifier
+; CHECK-NEXT:     Bitcode Writer
+
+define void @f() {
+  ret void
+}