[PATCH] D82705: [X86-64] Support Intel AMX instructions

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86RegisterInfo.td:268
 
+// Tile "registers". These aren't really registers. Or are they?
+def TMM0:  X86Reg<"tmm0",   0>;
----------------
Drop the "These aren't really registers. Or are they?"


================
Comment at: llvm/test/CodeGen/X86/AMX/amx-bf16-intrinsics.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile -mattr=+amx-bf16 -verify-machineinstrs | FileCheck %s
----------------
The intrinsics aren't in this patch so you can't test them.


================
Comment at: llvm/test/CodeGen/X86/AMX/amx-int8-intrinsics.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-int8 -verify-machineinstrs | FileCheck %s
----------------
Same here


================
Comment at: llvm/test/CodeGen/X86/AMX/amx-tile-intrinsics.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile -verify-machineinstrs | FileCheck %s
----------------
And here


================
Comment at: llvm/utils/TableGen/X86RecognizableInstr.cpp:649
+    // Operand 1 is a register operand in the R/M field.
+    HANDLE_OPTIONAL(roRegister) // FIXME: Why is this optional?
+    break;
----------------
It shouldn't be optional.


================
Comment at: llvm/utils/TableGen/X86RecognizableInstr.h:105
     AddCCFrm      = 9,
+    MRMDestMemFSIB = 31,
     MRMDestMem     = 32,
----------------
This needs to be rebased. The existing MRMDestMem is encoding 24 as of some recent changes.


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

https://reviews.llvm.org/D82705