[PATCH] D75203: [X86] Relax existing instructions to reduce the number of nops needed for alignment purposes

[Philip Reames via Phabricator via llvm-commits]

reames marked 3 inline comments as done.
reames added inline comments.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:661
+  auto &STI = *RF.getSubtargetInfo();
+  bool is16BitMode = STI.getFeatureBits()[X86::Mode16Bit];
+  return getRelaxedOpcode(Inst, is16BitMode) != Inst.getOpcode();
----------------
skan wrote:
> Is16BitMode
This code is copied from existing example.  Will change both after commit.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:802
+        // MCBoundaryAlignFragment(if exists) also marks the end of the branch.
+        for (int i = 0, N = BF.isFused() ? 2 : 1;
+             i != N && !isa<MCBoundaryAlignFragment>(F);
----------------
skan wrote:
> Capitalize i here otherwise clang tidy would report a warning
"i" and "I" are different variables.

Also, if clang-tidy reports a warning for "i", clang-tidy has a bug.  


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:811
+  // The layout is done. Mark every fragment as valid.
+  for (unsigned int i = 0, n = Layout.getSectionOrder().size(); i != n; ++i) {
+    MCSection &Section = *Layout.getSectionOrder()[i];
----------------
skan wrote:
> Capitalize i and N here otherwise clang tidy would report a warning
No, clang-tidy is wrong.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75203/new/

https://reviews.llvm.org/D75203