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

[Pengfei Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:104
+    bool DominateAllShape = true;
+    bool HasAMXBeforeCall = false;
+  };
----------------
LuoYuanke wrote:
> This looks to represent the tile register live through MBB in the code. Basically we need to reconfig after last call if the config register live out the MBB.
Yes, But it's equal to requiring instead of describing tile config register live into MBB.
```
    +----+
BB0 |Cfg |
    +----+
    /    \
+----+  +----+ 
|Call|  |AMX |
|AMX |  |Call|
+----+  +----+
  BB1     BB2
```
For example, tilecfg lives out BB0 and lives in BB1 and BB2, but we require it lives in for BB2 only. So,
```
BB1.NeedTileCfgLiveIn = false;
BB2.NeedTileCfgLiveIn = true;
```


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:140
+        : DFSPredsUpdater(PTC, MBB) {}
+    void update(MachineBasicBlock *MBB, MachineBasicBlock *Parent) {
+      if (PTC->BBVisitedInfo[MBB].DominateAllShape &&
----------------
LuoYuanke wrote:
> Maybe rename it as "update(MachineBasicBlock *Pred, MachineBasicBlock *MBB)"
How about `MBB` and `Succ`, I'd like to use MBB to respect the current node that is operating.
It's more clear to me if we consider `UpdateShapeDominators` is more independent with `DFSPredsUpdater`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99010