This is an archive of the discontinued LLVM Phabricator instance.

[X86] Support amx-bf16 intrinsic.
ClosedPublic

Authored by LiuChen3 on Feb 23 2021, 10:20 PM.

Details

Summary

Adding support for intrinsics of AMX-BF16.
This patch alse fix a bug that AMX-INT8 instructions will be selected with wrong
predicate.

Diff Detail

Event Timeline

LiuChen3 created this revision.Feb 23 2021, 10:20 PM
LiuChen3 requested review of this revision.Feb 23 2021, 10:20 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 23 2021, 10:20 PM
LiuChen3 updated this revision to Diff 325988.Feb 23 2021, 10:26 PM

Adding back 'avx512f' to amx-tile-basic.ll

pengfei added inline comments.Feb 23 2021, 11:20 PM
clang/lib/Headers/amxintrin.h
283

Is there much value to differentiate the type? We are using the same AMX type in the builtins. What do you think? @LuoYuanke

LuoYuanke added inline comments.Feb 23 2021, 11:32 PM
clang/lib/Headers/amxintrin.h
283

My first though is that we can reuse __tile1024i for bf16 tile for 2 reasons.

  1. We don't access the element of the tile.
  2. The destination element of amx-int8 is int32 and the destination element of amx-bf16 is float32, the element size is the same.
LiuChen3 added inline comments.Feb 23 2021, 11:36 PM
clang/lib/Headers/amxintrin.h
283

Does this means that user need to do explicitly type conversion?

pengfei added inline comments.Feb 23 2021, 11:39 PM
clang/lib/Headers/amxintrin.h
283

We don't allow user to use the tile in the structure. User should always use load/store intrinsic to pass there own data.

Harbormaster completed remote builds in B90538: Diff 325986.
LiuChen3 updated this revision to Diff 326002.Feb 24 2021, 12:23 AM

Address Pengfei and Yuanke's comments. We don't need more tile type.

pengfei accepted this revision.Feb 24 2021, 1:22 AM

LGTM.

This revision is now accepted and ready to land.Feb 24 2021, 1:22 AM

I don't know why pre-merge-checks failed. I can check-all successfully locally in redhat8. I don't have debian mainchine to reproduce this problem.

This revision was landed with ongoing or failed builds.Feb 24 2021, 5:07 PM
This revision was automatically updated to reflect the committed changes.
yubing added a subscriber: yubing.Mar 16 2021, 12:37 AM
yubing added inline comments.
clang/lib/Headers/amxintrin.h
340

Should we align this with "tile_dpbssd" by renaming it wth "tile_dpbf16ps"?

+1 first, didn't see key problems.

clang/lib/Headers/amxintrin.h
340

Yes, "t" already means "tile"