[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:193
+            MachineInstr *MI2 = MO2.getParent();
+            if (MI2->isMoveImmediate() || MI2->isPHI())
+              continue;
----------------
xiangzhangllvm wrote:
> xiangzhangllvm wrote:
> > pengfei wrote:
> > > 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.
> > I don't think  "Each phi will be iterated once."
> > 
> > MRI->def_operands(ShapeReg) only contains the shape reg used in AMX MI
> > 
> > 
> > ```
> > s1 s2
> >  \ /
> >   p1  s3  ...
> >    \  /   /
> >     p2   p3
> >      \  /
> >       p4
> >  then AMX MI use p4 as this shape,
> > ```
> > 
> >  only p4 in def_operands(ShapeReg).
> But here missed the def shape BB, it is not meet the following logic of use "shapeBBNumber == ShapeReachedCount" to judge the BB is all shape def reachable.
> 
> 
> ```
>    reg=imm        ...
>    /     \         / 
> amx          phi
>               |
>              BB   
> ```
> 
You are right, I excluded phi at first. I'll refactor for it. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99010