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

[Kan Shengchen via Phabricator via llvm-commits]

skan added inline comments.


================
Comment at: llvm/lib/MC/MCAssembler.cpp:1186-1187
+
+  for (iterator it = begin(), ie = end(); it != ie; ++it) {
+    MCSection &Sec = *it;
+    if (!Sec.getKind().isText())
----------------
Capitalize the first character of the variable it


================
Comment at: llvm/lib/MC/MCAssembler.cpp:1198-1200
+      if (F.getKind() ==  MCFragment::FT_Data)
+        // Skip and ignore
+        continue;
----------------
`F.getKind() ==  MCFragment::FT_Data || F.getKind() == FT_CompactEncodedInst`
I think we can handle `MCCompactEncodedInstFragment` here.


================
Comment at: llvm/lib/MC/MCAssembler.cpp:1214-1215
+      }
+      const unsigned OrigOffset = Layout.getFragmentOffset(&F);
+      const unsigned OrigSize = computeFragmentSize(Layout, F);
+      if (OrigSize == 0 || Relaxable.empty()) {
----------------
The return type of `getFragmentOffset` and `computeFragmentSize` is `uint64_t`, so I suggest use `uint64_t` here is better


================
Comment at: llvm/lib/MC/MCAssembler.cpp:1266-1267
+
+      const unsigned FinalOffset = Layout.getFragmentOffset(&F);
+      const unsigned FinalSize = computeFragmentSize(Layout, F);
+      assert(OrigOffset + OrigSize == FinalOffset + FinalSize &&
----------------
uint64_t


================
Comment at: llvm/test/MC/X86/align-via-relaxation.s:31
+
+  # Check that we're not shifting aroudn the offsets of labels - doing
+  # that would require a further round of relaxation
----------------
around


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

https://reviews.llvm.org/D75203