[PATCH] D23108: Implemented 132/213/231 forms selection for X86-FMA3-AVX512 opcodes.

[Craig Topper via llvm-commits]

craig.topper added inline comments.

================
Comment at: llvm/lib/Target/X86/Utils/X86InstrFMA3Info.cpp:264
@@ +263,3 @@
+void X86InstrFMA3Info::initGroupsOnceImpl() {
+  FMA3_AVX2_FULL_GROUP(VFMADD);
+  FMA3_AVX2_FULL_GROUP(VFMSUB);
----------------
These macros create a lot of small static tables and repeated initialization code for the map. Is there some way we can create fewer, larger static tables. Maybe a reg and mem table, a reg only table, and a mem only table with each row containing a group of opcodes and the attributes. Then loop through those larger tables to populate the map? Just a thought. Should reduce number of relocation entries and code size for this file.

Could probably be done as a follow up patch.

================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:5961
@@ -6390,1 +5960,3 @@
     case X86::VFMSUB231SSr_Int: case X86::VFNMSUB231SSr_Int:
+    case X86::VFMADD132SSZr_Int: case X86::VFNMADD132SSZr_Int:
+    case X86::VFMADD213SSZr_Int: case X86::VFNMADD213SSZr_Int:
----------------
This part isn't really related to the bigger change of introducing the new class and using it. And there are no test cases for this. Can this be split out with tests? Maybe find whatever test tests it for the VEX version and just add an AVX512 command line to it.

================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:5989
@@ -6412,1 +5988,3 @@
     case X86::VFMSUB231SDr_Int: case X86::VFNMSUB231SDr_Int:
+    case X86::VFMADD132SDZr_Int: case X86::VFNMADD132SDZr_Int:
+    case X86::VFMADD213SDZr_Int: case X86::VFNMADD213SDZr_Int:
----------------
Same as above comment.


https://reviews.llvm.org/D23108