[PATCH] D52174: [TableGen][SubtargetEmitter] Add the ability for processor models to describe dependency breaking instructions.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Some minors



================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:716
+    VPXORrr, VPANDNrr, VXORPSrr, VXORPDrr,
+    VXORPSYrr, VXORPDYrr, VANDNPSrr, VANDNPDrr,
+    VPSUBBrr, VPSUBDrr, VPSUBQrr, VPSUBWrr,
----------------
VANDNPSYrr/VANDNPDYrr?


================
Comment at: test/tools/llvm-mca/X86/BtVer2/zero-idioms-avx-256.s:68
+# CHECK-NEXT:  -      -      -     2.00    -     2.00    -      -      -      -      -      -      -      -     vaddps	%ymm0, %ymm0, %ymm1
+# CHECK-NEXT:  -      -      -      -     2.00    -     2.00    -      -      -      -      -      -      -     vxorps	%ymm1, %ymm1, %ymm1
+# CHECK-NEXT:  -      -      -     1.00   1.00   1.00   1.00    -      -      -      -      -      -      -     vblendps	$2, %ymm1, %ymm2, %ymm3
----------------
Shouldn't this only take a single resource cycle (0.5 rtp)? IIRC dep-breaking 256-bit ops only needs to process the upper half


================
Comment at: tools/llvm-mca/include/Instruction.h:319
   bool IsDepBreaking;
+  bool IsZeroIdiom;
 
----------------
Comments?


================
Comment at: tools/llvm-mca/lib/Stages/DispatchStage.cpp:115
   // be optimized at register renaming stage. That means, no physical register
   // is allocated to the instruction.
+  bool ShouldAllocateRegisters = !IS.isZeroIdiom();
----------------
Does this comment need updating to be zero-idiom specific?


================
Comment at: utils/TableGen/CodeGenSchedule.cpp:412
+void OpcodeInfo::addPredicateForProcModel(llvm::APInt CpuMask,
+                                          llvm::APInt OperandMask,
+                                          const Record *Predicate) {
----------------
Pass by const ref?


================
Comment at: utils/TableGen/CodeGenSchedule.h:311
+class OpcodeInfo {
+  llvm::SmallVector<PredicateInfo, 8> Predicates;
+
----------------
Why SmallVector here - the other added classes use std::vector


https://reviews.llvm.org/D52174