[X86] Fix compile time regression of D93594.

[LuoYuanke]

Summary

D93594 depend on the dominate tree and loop information. It increased
the compile time when build with -O0. However this is just to amend the
dominate tree and loop information, so that it is unnecessary to
re-analyze them again. Given the dominate tree of loop information are
absent in this pass, we can avoid amending them.

[LuoYuanke]

llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp

117–121

This is the key fix.

128

This is the key fix.

[LuoYuanke]

Fix commit message.

[LuoYuanke]

@nikic, do you know how to verify the patch fixed the compile time regression?

[LuoYuanke]

With "perf stat CLANG_BINARY -w -Werror=date-time -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DSQLITE_OMIT_LOAD_EXTENSION=1 -DSQLITE_THREADSAFE=0 -I. -MD -MT MultiSource/Applications/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o -MF MultiSource/Applications/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o.d -o MultiSource/Applications/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o -c ../MultiSource/Applications/sqlite3/sqlite3.c", I can see the sqlite3 is fixed in local test.

[nikic]

llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp

520

I believe you should still keep the addPreserved here. You do preserve it if it already exists.

520

I would create a DTU already here and pass it in (it's okay to create a DTU of nullptr). Otherwise the "lazy" strategy will actually be eager, as your DTU object is immediately destroyed.

[LuoYuanke]

llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp

519–523

Here I remove the dependency of dominator tree pass.

[LuoYuanke]

Address Nikita's comments.

[LuoYuanke]

llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp

520

Thanks. I'd prefer to not depending to DTU constructor given DT is nullptr, so I create DTU in X86LowerAMXIntrinsics constructor.

[nikic]

llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp

520

But aren't you still creating a DTU from nullptr, just in a different place now?

I'm okay with your version as well, but it seems more elegant to me to only pass around DTU rather than both DT and DTU. You can then also use it unconditionally, as DTU->applyUpdates() is a no-op if there is no actual dominator tree.

[LuoYuanke]

Address Nikita's comments.

[LuoYuanke]

llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp

520

Your suggestion looks good to me. Thank you! Patch updated.

[nikic]

LGTM, thanks for following up on this.

llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp

 
 static bool isV256I32Ty(Type *Ty) {
   if (auto *FVT = dyn_cast<FixedVectorType>(Ty))
     return FVT->getNumElements() == 256 &&
            FVT->getElementType()->isIntegerTy(32);
   return false;
 }
 
-static BasicBlock *createLoop(BasicBlock *Preheader, BasicBlock *Exit,
-                              Value *Bound, Value *Step, StringRef Name,
-                              IRBuilderBase &B, DomTreeUpdater &DTU, Loop *L,
-                              LoopInfo &LI) {
+namespace {
+class X86LowerAMXIntrinsics {
+  Function &Func;
+
+public:
+  X86LowerAMXIntrinsics(Function &F, DomTreeUpdater &DomTU, LoopInfo *LoopI)
+      : Func(F), DTU(DomTU), LI(LoopI) {}
+  bool visit();
+
+private:
+  DomTreeUpdater &DTU;
+  LoopInfo *LI;
+  BasicBlock *createLoop(BasicBlock *Preheader, BasicBlock *Exit, Value *Bound,
+                         Value *Step, StringRef Name, IRBuilderBase &B,
+                         Loop *L);
+  template <bool IsTileLoad>
+  Value *createTileLoadStoreLoops(BasicBlock *Start, BasicBlock *End,
+                                  IRBuilderBase &B, Value *Row, Value *Col,
+                                  Value *Ptr, Value *Stride, Value *Tile);
+  Value *createTileDPBSSDLoops(BasicBlock *Start, BasicBlock *End,
+                               IRBuilderBase &B, Value *Row, Value *Col,
+                               Value *K, Value *Acc, Value *LHS, Value *RHS);
+  template <bool IsTileLoad>
+  bool lowerTileLoadStore(Instruction *TileLoadStore);
+  bool lowerTileDPBSSD(Instruction *TileDPBSSD);
+  bool lowerTileZero(Instruction *TileZero);
+};
+
+BasicBlock *X86LowerAMXIntrinsics::createLoop(BasicBlock *Preheader,
+                                              BasicBlock *Exit, Value *Bound,
+                                              Value *Step, StringRef Name,
+                                              IRBuilderBase &B, Loop *L) {
   LLVMContext &Ctx = Preheader->getContext();
   BasicBlock *Header =
       BasicBlock::Create(Ctx, Name + ".header", Preheader->getParent(), Exit);
   BasicBlock *Body =
       BasicBlock::Create(Ctx, Name + ".body", Header->getParent(), Exit);
   BasicBlock *Latch =
       BasicBlock::Create(Ctx, Name + ".latch", Header->getParent(), Exit);
 
BasicBlock *X86LowerAMXIntrinsics::createLoop(BasicBlock *Preheader,
   DTU.applyUpdatesPermissive({
       {DominatorTree::Delete, Preheader, Tmp},
       {DominatorTree::Insert, Header, Body},
       {DominatorTree::Insert, Body, Latch},
       {DominatorTree::Insert, Latch, Header},
       {DominatorTree::Insert, Latch, Exit},
       {DominatorTree::Insert, Preheader, Header},
   });
-
-  L->addBasicBlockToLoop(Header, LI);
-  L->addBasicBlockToLoop(Body, LI);
-  L->addBasicBlockToLoop(Latch, LI);
+  if (LI) {
+    L->addBasicBlockToLoop(Header, *LI);
+    L->addBasicBlockToLoop(Body, *LI);
+    L->addBasicBlockToLoop(Latch, *LI);
+  }
   return Body;
 }
 
 template <bool IsTileLoad>
-static Value *createTileLoadStoreLoops(BasicBlock *Start, BasicBlock *End,
-                                       IRBuilderBase &B, DomTreeUpdater &DTU,
-                                       LoopInfo &LI, Value *Row, Value *Col,
-                                       Value *Ptr, Value *Stride, Value *Tile) {
+Value *X86LowerAMXIntrinsics::createTileLoadStoreLoops(
+    BasicBlock *Start, BasicBlock *End, IRBuilderBase &B, Value *Row,
+    Value *Col, Value *Ptr, Value *Stride, Value *Tile) {
   std::string IntrinName = IsTileLoad ? "tileload" : "tilestore";
-  Loop *RowLoop = LI.AllocateLoop();
-  Loop *ColLoop = LI.AllocateLoop();
+  Loop *RowLoop = nullptr;
+  Loop *ColLoop = nullptr;
+  if (LI) {
+    RowLoop = LI->AllocateLoop();
+    ColLoop = LI->AllocateLoop();
     RowLoop->addChildLoop(ColLoop);
-  if (Loop *ParentL = LI.getLoopFor(Start))
+    if (Loop *ParentL = LI->getLoopFor(Start))
       ParentL->addChildLoop(RowLoop);
     else
-    LI.addTopLevelLoop(RowLoop);
+      LI->addTopLevelLoop(RowLoop);
+  }
 
-  BasicBlock *RowBody =
-      createLoop(Start, End, Row, B.getInt16(1), IntrinName + ".scalarize.rows",
-                 B, DTU, RowLoop, LI);
+  BasicBlock *RowBody = createLoop(Start, End, Row, B.getInt16(1),
+                                   IntrinName + ".scalarize.rows", B, RowLoop);
   BasicBlock *RowLatch = RowBody->getSingleSuccessor();
 
-  BasicBlock *ColBody =
-      createLoop(RowBody, RowLatch, Col, B.getInt16(1),
-                 IntrinName + ".scalarize.cols", B, DTU, ColLoop, LI);
+  BasicBlock *ColBody = createLoop(RowBody, RowLatch, Col, B.getInt16(1),
+                                   IntrinName + ".scalarize.cols", B, ColLoop);
 
   BasicBlock *ColLoopLatch = ColBody->getSingleSuccessor();
   BasicBlock *ColLoopHeader = ColBody->getSinglePredecessor();
   BasicBlock *RowLoopHeader = RowBody->getSinglePredecessor();
   Value *CurrentRow = &*RowLoopHeader->begin();
   Value *CurrentCol = &*ColLoopHeader->begin();
   Type *EltTy = B.getInt32Ty();
   FixedVectorType *V256I32Ty = FixedVectorType::get(EltTy, 256);
if (IsTileLoad) {
     B.SetInsertPoint(ColBody->getTerminator());
     Value *Elt = B.CreateExtractElement(Vec, Idx);
 
     B.CreateStore(Elt, EltPtr);
     return nullptr;
   }
 }
 
-static Value *createTileDPBSSDLoops(BasicBlock *Start, BasicBlock *End,
-                                    IRBuilderBase &B, DomTreeUpdater &DTU,
-                                    LoopInfo &LI, Value *Row, Value *Col,
-                                    Value *K, Value *Acc, Value *LHS,
-                                    Value *RHS) {
-  Loop *RowLoop = LI.AllocateLoop();
-  Loop *ColLoop = LI.AllocateLoop();
-  Loop *InnerLoop = LI.AllocateLoop();
+Value *X86LowerAMXIntrinsics::createTileDPBSSDLoops(
+    BasicBlock *Start, BasicBlock *End, IRBuilderBase &B, Value *Row,
+    Value *Col, Value *K, Value *Acc, Value *LHS, Value *RHS) {
+  Loop *RowLoop = nullptr;
+  Loop *ColLoop = nullptr;
+  Loop *InnerLoop = nullptr;
+  if (LI) {
+    RowLoop = LI->AllocateLoop();
+    ColLoop = LI->AllocateLoop();
+    InnerLoop = LI->AllocateLoop();
     ColLoop->addChildLoop(InnerLoop);
     RowLoop->addChildLoop(ColLoop);
-  if (Loop *ParentL = LI.getLoopFor(Start))
+    if (Loop *ParentL = LI->getLoopFor(Start))
       ParentL->addChildLoop(RowLoop);
     else
-    LI.addTopLevelLoop(RowLoop);
+      LI->addTopLevelLoop(RowLoop);
+  }
 
-  BasicBlock *RowBody =
-      createLoop(Start, End, Row, B.getInt16(1), "tiledpbssd.scalarize.rows", B,
-                 DTU, RowLoop, LI);
+  BasicBlock *RowBody = createLoop(Start, End, Row, B.getInt16(1),
+                                   "tiledpbssd.scalarize.rows", B, RowLoop);
   BasicBlock *RowLatch = RowBody->getSingleSuccessor();
 
-  BasicBlock *ColBody =
-      createLoop(RowBody, RowLatch, Col, B.getInt16(1),
-                 "tiledpbssd.scalarize.cols", B, DTU, ColLoop, LI);
+  BasicBlock *ColBody = createLoop(RowBody, RowLatch, Col, B.getInt16(1),
+                                   "tiledpbssd.scalarize.cols", B, ColLoop);
   BasicBlock *ColLoopLatch = ColBody->getSingleSuccessor();
 
   B.SetInsertPoint(ColBody->getTerminator());
   BasicBlock *InnerBody =
       createLoop(ColBody, ColLoopLatch, K, B.getInt16(1),
-                 "tiledpbssd.scalarize.inner", B, DTU, InnerLoop, LI);
+                 "tiledpbssd.scalarize.inner", B, InnerLoop);
 
   BasicBlock *ColLoopHeader = ColBody->getSinglePredecessor();
   BasicBlock *RowLoopHeader = RowBody->getSinglePredecessor();
   BasicBlock *InnerLoopHeader = InnerBody->getSinglePredecessor();
   BasicBlock *InnerLoopLatch = InnerBody->getSingleSuccessor();
   Value *CurrentRow = &*RowLoopHeader->begin();
   Value *CurrentCol = &*ColLoopHeader->begin();
   Value *CurrentInner = &*InnerLoopHeader->begin();
Value *X86LowerAMXIntrinsics::createTileDPBSSDLoops(
   VecCPhiRowLoop->addIncoming(NewVecC, RowLatch);
   VecCPhiColLoop->addIncoming(NewVecC, ColLoopLatch);
   VecDPhiRowLoop->addIncoming(NewVecD, RowLatch);
   VecDPhiColLoop->addIncoming(NewVecD, ColLoopLatch);
 
   return NewVecD;
 }
 
-namespace {
-class X86LowerAMXIntrinsics {
-  Function &Func;
-
-public:
-  X86LowerAMXIntrinsics(Function &F, DominatorTree *DT, LoopInfo *LI)
-      : Func(F), DT(DT), LI(LI) {}
-  bool visit();
-
-private:
-  DominatorTree *DT;
-  LoopInfo *LI;
-  template <bool IsTileLoad>
-  bool lowerTileLoadStore(Instruction *TileLoadStore);
-  bool lowerTileDPBSSD(Instruction *TileDPBSSD);
-  bool lowerTileZero(Instruction *TileZero);
-};
-
 bool X86LowerAMXIntrinsics::lowerTileDPBSSD(Instruction *TileDPBSSD) {
   Value *M, *N, *K, *C, *A, *B;
   match(TileDPBSSD, m_Intrinsic<Intrinsic::x86_tdpbssd_internal>(
                         m_Value(M), m_Value(N), m_Value(K), m_Value(C),
                         m_Value(A), m_Value(B)));
-  DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
   Instruction *InsertI = TileDPBSSD;
   IRBuilder<> PreBuilder(TileDPBSSD);
   PreBuilder.SetInsertPoint(TileDPBSSD);
   // We visit the loop with (m, n/4, k/4):
   // %n_dword = lshr i16 %n, 2
   // %k_dword = lshr i16 %k, 2
   Value *NDWord = PreBuilder.CreateLShr(N, PreBuilder.getInt16(2));
   Value *KDWord = PreBuilder.CreateLShr(K, PreBuilder.getInt16(2));
   BasicBlock *Start = InsertI->getParent();
   BasicBlock *End =
-      SplitBlock(InsertI->getParent(), InsertI, DT, LI, nullptr, "continue");
+      SplitBlock(InsertI->getParent(), InsertI, &DTU, LI, nullptr, "continue");
   IRBuilder<> Builder(TileDPBSSD);
-  Value *ResVec = createTileDPBSSDLoops(Start, End, Builder, DTU, *LI, M,
-                                        NDWord, KDWord, C, A, B);
+  Value *ResVec =
+      createTileDPBSSDLoops(Start, End, Builder, M, NDWord, KDWord, C, A, B);
   // we cannot assume there always be bitcast after tiledpbssd. So we need to
   // insert one bitcast as required
   Builder.SetInsertPoint(End->getFirstNonPHI());
   Value *ResAMX =
       Builder.CreateBitCast(ResVec, Type::getX86_AMXTy(Builder.getContext()));
   // Delete tiledpbssd intrinsic and do some clean-up.
   for (auto UI = TileDPBSSD->use_begin(), UE = TileDPBSSD->use_end();
        UI != UE;) {
if (IsTileLoad)
     match(TileLoadStore,
           m_Intrinsic<Intrinsic::x86_tileloadd64_internal>(
               m_Value(M), m_Value(N), m_Value(Ptr), m_Value(Stride)));
   else
     match(TileLoadStore, m_Intrinsic<Intrinsic::x86_tilestored64_internal>(
                              m_Value(M), m_Value(N), m_Value(Ptr),
                              m_Value(Stride), m_Value(Tile)));
 
-  DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
   Instruction *InsertI = TileLoadStore;
   IRBuilder<> PreBuilder(TileLoadStore);
   PreBuilder.SetInsertPoint(TileLoadStore);
   Value *NDWord = PreBuilder.CreateLShr(N, PreBuilder.getInt16(2));
   Value *StrideDWord = PreBuilder.CreateLShr(Stride, PreBuilder.getInt64(2));
   BasicBlock *Start = InsertI->getParent();
   BasicBlock *End =
-      SplitBlock(InsertI->getParent(), InsertI, DT, LI, nullptr, "continue");
+      SplitBlock(InsertI->getParent(), InsertI, &DTU, LI, nullptr, "continue");
   IRBuilder<> Builder(TileLoadStore);
   Value *ResVec = createTileLoadStoreLoops<IsTileLoad>(
-      Start, End, Builder, DTU, *LI, M, NDWord, Ptr, StrideDWord,
+      Start, End, Builder, M, NDWord, Ptr, StrideDWord,
       IsTileLoad ? nullptr : Tile);
   if (IsTileLoad) {
     // we cannot assume there always be bitcast after tileload. So we need to
     // insert one bitcast as required
     Builder.SetInsertPoint(End->getFirstNonPHI());
     Value *ResAMX =
         Builder.CreateBitCast(ResVec, Type::getX86_AMXTy(Builder.getContext()));
     // Delete tileloadd6 intrinsic and do some clean-up
public:
   }
 
   bool runOnFunction(Function &F) override {
     TargetMachine *TM = &getAnalysis<TargetPassConfig>().getTM<TargetMachine>();
     if (!F.hasFnAttribute(Attribute::OptimizeNone) &&
         TM->getOptLevel() != CodeGenOpt::None)
       return false;
 
-    auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
-    auto &LI = getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
+    auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();
+    auto *DT = DTWP ? &DTWP->getDomTree() : nullptr;
+    auto *LIWP = getAnalysisIfAvailable<LoopInfoWrapperPass>();
+    auto *LI = LIWP ? &LIWP->getLoopInfo() : nullptr;
+    DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
 
-    X86LowerAMXIntrinsics LAT(F, &DT, &LI);
+    X86LowerAMXIntrinsics LAT(F, DTU, LI);
     return LAT.visit();
   }
   StringRef getPassName() const override { return "Lower AMX intrinsics"; }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.addRequired<DominatorTreeWrapperPass>();
     AU.addPreserved<DominatorTreeWrapperPass>();
-    AU.addRequired<LoopInfoWrapperPass>();
     AU.addPreserved<LoopInfoWrapperPass>();
     AU.addRequired<TargetPassConfig>();
   }
 };
 
 } // anonymous namespace
 
 static const char PassName[] = "Lower AMX intrinsics";
 char X86LowerAMXIntrinsicsLegacyPass::ID = 0;
 INITIALIZE_PASS_BEGIN(X86LowerAMXIntrinsicsLegacyPass, DEBUG_TYPE, PassName,
                       false, false)
-INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
 INITIALIZE_PASS_END(X86LowerAMXIntrinsicsLegacyPass, DEBUG_TYPE, PassName,
                     false, false)
 
 FunctionPass *llvm::createX86LowerAMXIntrinsicsPass() {
   return new X86LowerAMXIntrinsicsLegacyPass();
 }

llvm/test/CodeGen/X86/O0-pipeline.ll

 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
 ; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT:     Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Expand Atomic instructions
-; CHECK-NEXT:       Dominator Tree Construction
-; CHECK-NEXT:       Natural Loop Information
 ; CHECK-NEXT:       Lower AMX intrinsics
 ; CHECK-NEXT:       Lower AMX type for load/store
 ; CHECK-NEXT:       Module Verifier
 ; CHECK-NEXT:       Lower Garbage Collection Instructions
 ; CHECK-NEXT:       Shadow Stack GC Lowering
 ; CHECK-NEXT:       Lower constant intrinsics
 ; CHECK-NEXT:       Remove unreachable blocks from the CFG
 ; CHECK-NEXT:       Scalarize Masked Memory Intrinsics

llvm/test/CodeGen/X86/opt-pipeline.ll

 ; CHECK-NEXT: Assumption Cache Tracker
 ; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT:     Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Expand Atomic instructions
-; CHECK-NEXT:       Dominator Tree Construction
-; CHECK-NEXT:       Natural Loop Information
 ; CHECK-NEXT:       Lower AMX intrinsics
 ; CHECK-NEXT:       Lower AMX type for load/store
 ; CHECK-NEXT:       Module Verifier
+; CHECK-NEXT:       Dominator Tree Construction
 ; CHECK-NEXT:       Basic Alias Analysis (stateless AA impl)
+; CHECK-NEXT:       Natural Loop Information
 ; CHECK-NEXT:       Canonicalize natural loops
 ; CHECK-NEXT:       Scalar Evolution Analysis
 ; CHECK-NEXT:       Loop Pass Manager
 ; CHECK-NEXT:         Canonicalize Freeze Instructions in Loops
 ; CHECK-NEXT:         Induction Variable Users
 ; CHECK-NEXT:         Loop Strength Reduction
 ; CHECK-NEXT:       Basic Alias Analysis (stateless AA impl)
 ; CHECK-NEXT:         Function Alias Analysis Results