[PATCH] D125075: [X86][AMX] Multiple configure for AMX register.

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FastPreTileConfig.cpp:550
+    if (HasTileOperand(MRI, MI))
+      HasUnconfigTile = true;
+    // According to AMX ABI, all the tile registers including config register
----------------
I am afraid , even without call, 1 ldtilecfg is not enough for 1 MBB.
For example
In 1 MBB, it contain 4 shapes, but the first 3 shape used up the "max reg num of ldtilecfg (8)" virtual tile regs in follow code. So It need another ldtilecfg.


================
Comment at: llvm/lib/Target/X86/X86FastPreTileConfig.cpp:564
+    if (!isTileDef(MRI, MI))
+      continue;
+    // If MI dominate the last shape def instruction, we need insert
----------------
Not much sure here must no "COPY" for tile.  (T1 = Copy T0)
Better to add assert here to exclude unexpected Tile def (MI is not intrinsic).


================
Comment at: llvm/lib/Target/X86/X86FastPreTileConfig.cpp:574
+    if (LastShapeMI && dominates(MBB, MI, LastShapeMI))
+      Config(*(++LastShapeMI->getIterator()));
+    MachineOperand *RowMO = &MI.getOperand(1);
----------------
Sorry, In my understand, the comment is not match the code. 
it should be

```
    //   tilezero
    //   def row
    //   def col  <- LastShapeMI 
   //   ldtilecfg <- insert
    //   tilezero(row, col)
```


================
Comment at: llvm/lib/Target/X86/X86FastPreTileConfig.cpp:604
+        // reload befor UseMI
+        reload(UseMI.getIterator(), TileReg, RowMO, ColMO);
+      } else {
----------------
We should escape duplicated reload. for example:

Not re-gen reload for line 2.
```
1  T0 = TileLoad
2   TileUse T0
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125075