Page MenuHomePhabricator

[X86][AMX] Refactor for PostRA ldtilecfg pass.
ClosedPublic

Authored by pengfei on Apr 6 2021, 9:06 AM.

Details

Summary

This is a follow up of D99010. We didn't consider the live range of shape registers when hoist ldtilecfg. There maybe risks, e.g. we happen to insert it to an invalid range of some registers and get unexpected error.

This patch fixes this problem by storing the value to corresponding stack place of ldtilecfg after all its definition immediately.

This patch also fix a problem in previous code: If we don't have a ldtilecfg which dominates all AMX instructions, we cannot initialize shapes for other ldtilecfg.

There're still some optimization points left. E.g. eliminate unused mov instructions, break the def-use dependency before RA etc.

Diff Detail

Event Timeline

pengfei created this revision.Apr 6 2021, 9:06 AM
pengfei requested review of this revision.Apr 6 2021, 9:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2021, 9:06 AM
pengfei updated this revision to Diff 336611.Apr 10 2021, 6:59 AM

A workable implementation.

pengfei retitled this revision from [X86][AMX][WIP] Refactor for PostRA ldtilecfg pass. to [X86][AMX] Refactor for PostRA ldtilecfg pass..Apr 10 2021, 7:02 AM
pengfei edited the summary of this revision. (Show Details)
pengfei updated this revision to Diff 336613.Apr 10 2021, 7:15 AM

Add comments for shape layout.

xiangzhangllvm added inline comments.Apr 11 2021, 6:02 PM
llvm/lib/Target/X86/X86TileConfig.cpp
176

The problem is the DefMI may comes from a reload, at its live range may not include the "shape_config_write" instructions which you want generated.

pengfei added inline comments.Mon, Apr 12, 10:45 PM
llvm/lib/Target/X86/X86TileConfig.cpp
176

We insert MOV8mi/MOV8mr adjoining any DefMI, so live range is not a problem.
We may insert redundant store instructions for reload, but it won't result in correctness problems.

llvm/lib/Target/X86/X86TileConfig.cpp
124

I see here suppose all virtual tile regs are same shape, how we handle the cfg for tileload/store at spill ?

pengfei added inline comments.Mon, Apr 12, 11:58 PM
llvm/lib/Target/X86/X86TileConfig.cpp
124

The assumption is all virtual regs that bound to the same physical regs are the same shape. We don't handle cfg when spill.

xiangzhangllvm accepted this revision.Tue, Apr 13, 12:37 AM

LGTM, (spill will only happen in the same shape tile register.)

This revision is now accepted and ready to land.Tue, Apr 13, 12:37 AM
LuoYuanke added inline comments.Tue, Apr 13, 7:30 AM
llvm/lib/Target/X86/X86TileConfig.cpp
111

Would you add comments to explain in PreRA we have inserted palette id in the entry?

pengfei updated this revision to Diff 337302.Tue, Apr 13, 6:13 PM
pengfei marked an inline comment as done.

Address Yuanke's comment.

LuoYuanke accepted this revision.Tue, Apr 13, 6:19 PM

LGTM. Thank you!

This revision was landed with ongoing or failed builds.Tue, Apr 13, 7:08 PM
This revision was automatically updated to reflect the committed changes.