[PATCH] D39840: [MC][X86] Code padding for performance stability - Branch instructions and targets alignment

[Gadi Haber via Phabricator via llvm-commits]

gadi.haber added inline comments.


================
Comment at: lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:77
   const uint64_t MaxNopLength;
+
 public:
----------------
why is there a diff here? Did some unseen character was inserted?


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCCodePadder.cpp:38
+      CPU != "core-avx-i" && CPU != "haswell" && CPU != "core-avx2" &&
+      CPU != "broadwell" && CPU != "skylake")
+    return;
----------------
isn't it safer just to check that the CPU is atom instead?
The rule is good for any future CPU by default.


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCCodePadder.h:23
+
+/// The X86-specific class incharge of all code padding decisions for the X86
+/// target.
----------------
zvi wrote:
> *in charge
decisions --> policies ?


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCCodePadder.h:41
+
+/// A padding policy that handles brach instructions (all types of jmps and
+/// calls) and the first instruction after a branch (i.e. first instruction in a
----------------
*branch


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCCodePadder.h:107
+  ///
+  /// \returns true iff \p Inst is a branch (all types of jmps and calls).
+  bool instructionRequiresPaddingFragment(const MCInst &Inst) const override;
----------------
both direct and indirect?


Repository:
  rL LLVM

https://reviews.llvm.org/D39840