[X86][AMX] Hoist ldtilecfg

[pengfei]

Summary

The previous code calculated the first ldtilecfg by dominating all AMX registers' def. This may result in the ldtilecfg being inserted into a loop.

This patch try to calculate the nearest point where post dominats all shapes of AMX registers.

[pengfei]

Minor fix.
Add a test to verify the sink code.

[pengfei]

llvm/test/CodeGen/X86/AMX/amx-ldtilecfg-insert.ll

139

This seems another bug, I'll investigate it.

[pengfei]

Fix a bug found by test.

[pengfei]

llvm/test/CodeGen/X86/AMX/amx-ldtilecfg-insert.ll

139

Fixed.

[pengfei]

Refactor.

[pengfei]

Fixed the problem when the sink need to be forked. I.e.

     +------+
     |Entry | BB0
     +------+
     /     \
+------+  +------+
|Shape1|  |Shape2| BB2
+------+  +------+
BB1  \       /
     +------+
     | AMX  | BB3
     +------+

If BB1 and BB2 don't have a call, we will try to insert ldtilecfg from BB0.
Since BB0 doesn't dominate all shapes, we will sink the insert point to its successors.
The previous code has limitation that only one of BB0's successors has NeedTileCfgLiveIn = true.
This update fixes the problem without major changes. So I amended it instead of putting into a new revision.

[pengfei]

The algorithm for updating shape postdominate BBs is buggy. For a given ShapeBB, clear all its predecessors flag is not enough since its unreachable BBs are also need to clear.
I have thought out a new method but need major refactor. Stay tuned~

[pengfei]

The algorithm for updating shape postdominate BBs is buggy. For a given ShapeBB, clear all its predecessors flag is not enough since its unreachable BBs are also need to clear.

I have thought out a new method but need major refactor. Stay tuned~

Worked it out.

[pengfei]

A litter format & comments refactor.

[pengfei]

Fix a bug in collectShapeInfo.

[LuoYuanke]

llvm/lib/Target/X86/X86PreTileConfig.cpp

16–18

Do you mean all tile registers are caller saved?

368–369

Is this semicolon necessary?

371

May update the comments. All shapes are reachable.

375

Do we need to recursively handle phi instruction? In below example, record "def shape" instruction?

def shape

\
phi
  \
 phi

377

So we would try to insert ldtilecfg at the entry if there is not shape dependency issue?

382

Can we sink the ldtilecfg in below case? We need insert ldtilecfg to B4.

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

[pengfei]

Address Yuanke's comments.

llvm/lib/Target/X86/X86PreTileConfig.cpp

16–18

No, tile registers are visible to RA, we don't need to restore them ourselves.
We are mainly doing the restore for the shape register in this pass.

368–369

Good catch. It's residue from previous lambda expression.

375

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.

377

Yes. Here the insert points are candidates. We will sink them based on the shape defs. We will insert at the entry if shape dependency satisfies.

382

Yes. We will sink two candidates from B1 anf B2. (If there's no call in B2, the candidates are B1 and Entry) We avoid BB being multi visited. So the second sink will stop at B3. This first sink will cross B3 and reach B4.

[xiangzhangllvm]

llvm/lib/Target/X86/X86PreTileConfig.cpp

55

use C++ comments // between line 54,55

67

use getMI() or directly p->MI is more readable.

210

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.

370

here may be a constant imm in PHI's incomings.

375

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.

[xiangzhangllvm]

llvm/lib/Target/X86/X86PreTileConfig.cpp

379

Not sure the non-bottom BBs in loop can both with "fall through + continue".

[pengfei]

Address Xiang's comments.

llvm/lib/Target/X86/X86PreTileConfig.cpp

210

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.

370

Seems IR parser will normalize this case, so we don't need to handle it.

375

1. The PostRA pass can handle this case.
2. We have covered this case here.

[pengfei]

llvm/lib/Target/X86/X86PreTileConfig.cpp

379

non-bottom BBs can propagate until the bottom BB.

[xiangzhangllvm]

llvm/lib/Target/X86/X86PreTileConfig.cpp

210

Yes, PHI is exception, the exceptions is very few, more better than list all the AMX opcodes.

375

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).

375

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

[pengfei]

llvm/lib/Target/X86/X86PreTileConfig.cpp

375

You are right, I excluded phi at first. I'll refactor for it. Thanks!

[xiangzhangllvm]

llvm/lib/Target/X86/X86PreTileConfig.cpp

377

I am a little puzzle here, The I.MBB is all shape def reachable, but how can we make sure the BBVisitedInfo[I.MBB].LastShape is shape def reachable ? They may be not same BB.

[pengfei]

llvm/lib/Target/X86/X86PreTileConfig.cpp

377

LastShape is the last shape def in a BB. It is only used when its BB is all shape def reachable. I.e. insert ldtilecfg after it. But it can be null if the BB has no shape def, which means we can insert at the begining of the BB.

[pengfei]

Address Xiang's comment.
Iterate all PHIs recursively to find all shapes.

[LuoYuanke]

llvm/lib/Target/X86/X86PreTileConfig.cpp

372

Better to add test case to cover the logic.

377

Better to add test case for it.

[pengfei]

Address Yuanke's comments.

Add test5 for checking shape peek's loop break.
Add test6 for checking shape reachability's loop break.

[pengfei]

Fix a silly bug that uses != in case == and a bug when records shape for phi.

This is found by work with PostRA implementation. This should be found by case amx-ldtilecfg-insert.ll:, but happened to get expected output with these 2 bugs.

[pengfei]

Fix crush when build with -g.

[pengfei]

Add back a condition deleted last time by mistaken.

[LuoYuanke]

llvm/lib/Target/X86/X86PreTileConfig.cpp

163

!MI.isPseudo()?

[xiangzhangllvm]

llvm/lib/Target/X86/X86PreTileConfig.cpp

374

This should be calculated in ShapeNum, for AMX always use reg as its shape after PHI.

[pengfei]

Refactor for excluding DBG_VALUE case.

llvm/lib/Target/X86/X86PreTileConfig.cpp

163

Yeah, I meant to. Surprisingly, we didn't set this flag when we define PseudoI. So the tests happened to pass. :)

374

The immediate value is specially handled in PostRA pass. Since the immediate value is always defined adjoining its use, we cannot calculate it as shape def. Otherwise, they will intersect with other AMX instructions.

[LuoYuanke]

LGTM. Thanks!