This is an archive of the discontinued LLVM Phabricator instance.

[X86-64] Support Intel AMX instructions
ClosedPublic

Authored by craig.topper on Jun 27 2020, 11:44 PM.

Details

Summary

INTEL ADVANCED MATRIX EXTENSIONS (AMX).
AMX is a new programming paradigm, it has a set of 2-dimensional registers
(TILES) representing sub-arrays from a larger 2-dimensional memory image and
operate on TILES.

Spec can be found in Chapter 3 here https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJun 27 2020, 11:44 PM
craig.topper edited the summary of this revision. (Show Details)
craig.topper added inline comments.Jun 28 2020, 10:34 AM
llvm/lib/Target/X86/X86RegisterInfo.td
268

Drop the "These aren't really registers. Or are they?"

llvm/test/CodeGen/X86/AMX/amx-bf16-intrinsics.ll
1 ↗(On Diff #273902)

The intrinsics aren't in this patch so you can't test them.

llvm/test/CodeGen/X86/AMX/amx-int8-intrinsics.ll
1 ↗(On Diff #273902)

Same here

llvm/test/CodeGen/X86/AMX/amx-tile-intrinsics.ll
1 ↗(On Diff #273902)

And here

llvm/utils/TableGen/X86RecognizableInstr.cpp
651

It shouldn't be optional.

llvm/utils/TableGen/X86RecognizableInstr.h
105

This needs to be rebased. The existing MRMDestMem is encoding 24 as of some recent changes.

xiangzhangllvm marked 3 inline comments as done.Jun 29 2020, 3:23 AM

Done, but I find the last llvm code can not pass "make check-all", no relation with this patch.

craig.topper added inline comments.Jun 29 2020, 5:50 PM
llvm/lib/Target/X86/X86RegisterInfo.td
637

I think this should be just TILE. The V seems unnecessary and in every other case means Vector. This is more like a matrix so vector doesn't make sense

Change VTILE to TILE

This revision is now accepted and ready to land.Jun 29 2020, 7:39 PM

3 case failed

LLVM :: MC/Disassembler/X86/simple-tests.txt
LLVM :: MC/Disassembler/X86/x86-16.txt
LLVM :: MC/Disassembler/X86/x86-32.txt

echo "0x0f 0xae 0xe8" | llvm-mc --disassemble -triple=i686-apple-darwin9
should print "lfence"
but print "warning: invalid instruction encoding"

something changed the "modRMTable" in TD file "X86GenDisassemblerTables.inc"
I am trying to find the reason.

pengfei added inline comments.Jun 30 2020, 8:07 AM
llvm/utils/TableGen/X86RecognizableInstr.cpp
804

You missed break here. That's the reason it failed.

craig.topper commandeered this revision.Jun 30 2020, 3:44 PM
craig.topper edited reviewers, added: xiangzhangllvm; removed: craig.topper.

Commandeering to fix a disassembler issue where we would crash if the VEX.vvvv field is set to 0x8-0xf(after inverting). Now we report it as an invalid encoding.

This revision now requires review to proceed.Jun 30 2020, 3:44 PM

-Fix the previously noted missing break
-Prevent trying to create a register encoding past tmm7

-Fix the previously noted missing break
-Prevent trying to create a register encoding past tmm7

OK

llvm/utils/TableGen/X86RecognizableInstr.cpp
804

Thank you very much!!

LuoYuanke added inline comments.Jun 30 2020, 6:36 PM
llvm/test/MC/X86/AMX/x86-64-amx-error.s
9

Do we have an error case that test the register number should be constrained to tmm0 ~ tmm7? Both for assembly and disassembly?

xiangzhangllvm accepted this revision.Jun 30 2020, 6:52 PM
This revision is now accepted and ready to land.Jun 30 2020, 6:52 PM

check-llvm passed.
LGTM

craig.topper marked 2 inline comments as done.Jun 30 2020, 7:40 PM
craig.topper added inline comments.
llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp
783

Here's the range check for tmm0-tmm7 I added

llvm/test/MC/X86/AMX/x86-64-amx-error.s
9

The assembler can’t parse a register that doesnt have a name. Only tmm0-7 are defined.

I fixed the disassembler earlier today.

Hello, Craig, can I commit it now?

Hello, Craig, can I commit it now?

Yes

This revision was automatically updated to reflect the committed changes.
MaskRay added a subscriber: MaskRay.Jul 1 2020, 9:54 PM

@xiangzhangllvm

Hi, your git commit contains extra Phabricator tags. You can drop Reviewers: Subscribers: Tags: and the text Summary: from the git commit with the following script:

arcfilter () {
        arc amend
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people. Please keep the tag. (--date=now is my personal preference (author dates are usually not useful. Using committer dates can make log almost monotonic in time))

llvm/utils/git/pre-push.py can validate the message does not include unneeded tags.

xiangzhangllvm added a comment.EditedJul 1 2020, 10:23 PM

I tried it, it just remove tags, not tags with their info, I miss-understand at first, thank you!