[PATCH] D34393: Adding code padding for performance stability - infrastructure

[Gadi Haber via Phabricator via llvm-commits]

gadi.haber added inline comments.


================
Comment at: include/llvm/CodeGen/AsmPrinter.h:630
                                 const GlobalIndirectSymbol& GIS);
+  void setupPaddingContext(const MachineBasicBlock &MBB,
+                           MCCodePaddingContext &Context) const;
----------------
Consider to rename the method to: setupCodePaddingContext


================
Comment at: include/llvm/MC/MCAsmBackend.h:153
+                                        const MCCodePaddingContext &Context);
+  /// Handles all target related code padding when done writing a block to an
+  /// object file.
----------------
when --> after


================
Comment at: include/llvm/MC/MCCodePadder.h:24
+class MCObjectStreamer;
+class MCPaddingFragment;
+class MCSection;
----------------
MCPaddingFragment --> MCCodePaddingFragment ?


================
Comment at: include/llvm/MC/MCCodePadder.h:47
+  /// All the supported MCCodePaddingPolicies.
+  SmallPtrSet<MCCodePaddingPolicy *, 4> PaddingPolicies;
+
----------------
PaddingPolicies --> CodePaddingPolicies ?


================
Comment at: include/llvm/MC/MCCodePadder.h:216
+  virtual bool
+  basicBlockRequiresPaddingFragment(const MCCodePaddingContext &Context) const {
+    return false;
----------------
basicBlockRequiresPaddingFragment --> isBasicBlockRequiresPaddingFragment ?


================
Comment at: include/llvm/MC/MCCodePadder.h:224
+  /// \returns true iff this policy needs padding for \p Inst.
+  virtual bool instructionRequiresPaddingFragment(const MCInst &Inst) const {
+    return false;
----------------
instructionRequiresPaddingFragment --> isInstructionRequiresPaddingFragment 


================
Comment at: include/llvm/MC/MCCodePadder.h:236
+  /// \returns the penalty caused by \p Range.
+  double computeRangePenaltyWeight(const MCPFRange &Range, uint64_t Offset,
+                                   MCAsmLayout &Layout) const;
----------------
computeRangePenaltyWeight --> computeScopePenaltyWeight ?


================
Comment at: include/llvm/MC/MCFragment.h:332
+///
+class MCPaddingFragment : public MCFragment {
+  /// A mask containing all the kinds relevant to this fragment. i.e. the i'th
----------------
MCPaddingFragment --> MCCodePaddingFragment


================
Comment at: include/llvm/MC/MCFragment.h:335
+  /// bit will be set iff kind i is relevant to this fragment.
+  uint64_t PaddingPoliciesMask;
+  /// A boolean indicating if this fragment will actually hold padding. If its
----------------
PaddingPoliciesMask --> AggregatedPaddingPoliciesMask ?


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2607
+  assert(LI != nullptr && "Loop info must be valid");
+  Context.IsPaddingActive = !MF->hasInlineAsm() &&
+                            !MF->getFunction()->optForSize() &&
----------------
IsPaddingActive  --> IsCodePaddingActive ?


https://reviews.llvm.org/D34393