[PATCH] D97982: [MC] Introduce NeverAlign fragment type

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

The patch should come with with tests.

We probably want to avoid another feature which does complex things in `emitInstructionBegin`/`emitInstructionEnd`, so (1) helping just inline asm does not seem a good justification to me. Perhaps (2) should be used.



================
Comment at: llvm/include/llvm/MC/MCFragment.h:338
+class MCNeverAlignFragment : public MCFragment {
+  /// Alignment - The alignment the end of the next fragment should avoid
+  unsigned Alignment;
----------------
Don’t duplicate function or class name at the beginning of the comment.

https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments


================
Comment at: llvm/lib/MC/MCAssembler.cpp:357
+    if (const auto *NextFrag = dyn_cast<MCRelaxableFragment>(F.getNextNode())) {
+      OffsetToAvoid = NAF.getAlignment() -
+                      (NextFrag->getContents().size() % NAF.getAlignment());
----------------
The logic is wrong if NextFrag->getContents().size() % NAF.getAlignment() == 0.
In that case you want to make the value 0


================
Comment at: llvm/lib/MC/MCAssembler.cpp:363
+                      (NextFrag->getContents().size() % NAF.getAlignment());
+    }
+    // Check if the current offset matches the alignment plus offset we want to
----------------
If neither, return 0


================
Comment at: llvm/lib/MC/MCAssembler.cpp:370
+      if (Size > 0 && NAF.hasEmitNops()) {
+        while (Size % getBackend().getMinimumNopSize())
+          Size += 1;
----------------
Store Size % getBackend().getMinimumNopSize() into a variable, check whether it is zero, instead of using a loop which increases size one at once.


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

https://reviews.llvm.org/D97982