[PATCH] D99010: [X86][AMX] Hoist ldtilecfg

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:16
 ///
-///  is transformed to
+/// The shape register is caller saved according to ABI. We need to insert
+/// ldtilecfg again after the call instruction if callee clobbers any AMX
----------------
Do you mean all tile registers are caller saved?


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:193
+            MachineInstr *MI2 = MO2.getParent();
+            if (MI2->isMoveImmediate() || MI2->isPHI())
+              continue;
----------------
Do we need to recursively handle phi instruction? In below example, record "def shape" instruction?

def shape
    \
    phi
      \
     phi


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:253
+        if (Pred == &MF.front())
+          CfgNeedInsert.insert(MIRef(Pred));
+        else
----------------
So we would try to insert ldtilecfg at the entry if there is not shape dependency issue?


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:294
+        if (BBVisitedInfo[I.MBB].ShapeReachedCount == ShapeBBNum) {
+          // If find a BB postdominates all shapes, stop sink and try to insert.
+          InsertPoints.insert(I);
----------------
May update the comments. All shapes are reachable.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:305
+          for (auto *Succ : I.MBB->successors())
+            if (BBVisitedInfo[Succ].NeedTileCfgLiveIn)
+              WorkList.push_back(MIRef(Succ));
----------------
Can we sink the ldtilecfg in below case? We need insert ldtilecfg to B4.

```
                Entry
              /         \
            B1         B2
           call        call
              \           /
                  B3
                    |
               B4 shape defs
                    |
                 AMX

```


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:329
     }
-    if (!UsableRegs.none())
-      addFrameReference(BuildMI(*I->getParent(), ++I->getIterator(), DebugLoc(),
-                                TII->get(X86::LDTILECFG)),
-                        FI);
-  }
-}
+  };
 
----------------
Is this semicolon necessary?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99010/new/

https://reviews.llvm.org/D99010