[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:108
+  /// Check if MI is AMX instruction.
+  bool isAMXInstruction(MachineInstr &MI) {
+    switch (MI.getOpcode()) {
----------------
xiangzhangllvm wrote:
> I suggest use tile operand to judge if a MI is AMX Instruction or not. and spilt the collection of shape to another special function.
But we still need to judge if it is PHI. I don't know if there're other exceptions.
We have split it to collectShapeInfo.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:188
+        for (unsigned Index = 0; Index < MI->getNumOperands() / 2; ++Index) {
+          Register DefReg = MI->getOperand(Index * 2 + 1).getReg();
+          for (MachineOperand &MO2 : MRI->def_operands(DefReg)) {
----------------
xiangzhangllvm wrote:
> here may be a constant imm in PHI's incomings.
Seems IR parser will normalize this case, so we don't need to handle it.


================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:193
+            MachineInstr *MI2 = MO2.getParent();
+            if (MI2->isMoveImmediate() || MI2->isPHI())
+              continue;
----------------
xiangzhangllvm wrote:
> pengfei wrote:
> > LuoYuanke wrote:
> > > Do we need to recursively handle phi instruction? In below example, record "def shape" instruction?
> > > 
> > > def shape
> > >     \
> > >     phi
> > >       \
> > >      phi
> > I don't think so. Each phi will be iterated once. So we just need to peek its latest level and ignore the phi in it.
> > ```
> > s1 s2
> >  \ /
> >   p1  s3  ...
> >    \  /   /
> >     p2   p3
> >      \  /
> >       p4
> > ```
> > For exampe, if we meet p4 firstly, we find all its defs are phis, we don't do anything. Then, we meet p2 in another iteration and record s3. Then p1 etc. The order doesn't metter, you can see all the real shape defs will be recorded without recursion.
> 1) MI2->isMoveImmediate() may need to be handled, because the imm may always keep in a register after PHI.
> In other wards, if AMX MI use the imm shape, they must use the register after PHI. 
> 
> 2) the recursive PHI case is very rare, we can let it easy, not handle it, there is not correction problem.
1. The PostRA pass can handle this case.
2. We have covered this case here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99010