[PATCH] D72225: Align branches within 32-Byte boundary(Prefix padding)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/MC/MCAssembler.cpp:1029
+           "The fragment doesn't have any instruction.");
+    assert(computeFragmentSize(Layout, *(BF.getNextNode())) <= 15 &&
+           "The fragment's size must be no longer than 15 since it should only "
----------------
This 15 limit is X86 specific. Seems weird to have it mentioned in a target independent file. Can we abstract this somehow? It can probably happen after the branch.


================
Comment at: llvm/lib/MC/MCAssembler.cpp:1033
+    NewSize = std::min({NewSize,
+                        15 - computeFragmentSize(Layout, *(BF.getNextNode())),
+                        static_cast<uint64_t>(BF.getMaxBytesToEmit())});
----------------
Same with this 15.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:412
+         "Prefixes can be added only in 32-bit or 64-bit mode.");
+  for (const auto &Operand : Inst) {
+    if (Operand.isReg())
----------------
I think this loop picks up instructions that don't use segment registers for just memory. And probably picks the wrong prefix for this instruction

mov %fs:0x1, %ss

The loop will find the %ss destination register first. But the %fs is the correct segment to use.


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

https://reviews.llvm.org/D72225