[PATCH] D27901: [X86][[AVX512] Code size reduction in X86 by replacing EVEX with VEX encoding

[Zvi Rackover via Phabricator via llvm-commits]

zvi added inline comments.


================
Comment at: lib/Target/X86/X86EvexToVex.cpp:100
+bool EvexToVexInstPass::runOnMachineFunction(MachineFunction &MF) {
+  TII = MF.getSubtarget<X86Subtarget>().getInstrInfo();
+  bool rc = false;
----------------
Maybe bail-out early if subtarget does not support avx512?
   const X86Subtarget &ST = MF.getSubtarget<X86Subtarget>();
   if (!ST.hasAVX512())
     return false;


================
Comment at: lib/Target/X86/X86EvexToVex.cpp:133
+
+    //const MCInstrDesc &Desc = I->getDesc();
+
----------------
Please remove the comment


================
Comment at: lib/Target/X86/X86EvexToVex.cpp:141
+
+    // Check for EVEX instructions with mask or broadcast.
+    if (Desc.TSFlags & (X86II::EVEX_K | X86II::EVEX_B))
----------------
I think it would be helpful if the comment said that in these cases the transformation is not possible because EVEX is needed to carry this information.


================
Comment at: lib/Target/X86/X86EvexToVex.cpp:199
+
+    for (const MachineOperand &MO : MI.operands()) {
+      if (!MO.isReg())
----------------
An improvement may be to iterate only over MI.explicit_operands()


Repository:
  rL LLVM

https://reviews.llvm.org/D27901