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

[Kan Shengchen via Phabricator via llvm-commits]

skan marked an inline comment as done.
skan added inline comments.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:608
+    if (isMacroFused(PrevInst, Inst)) {
+      if (isa_and_nonnull<MCAlignFragment>(PF->getPrevNode()))
+        return;
----------------
LuoYuanke wrote:
> I notice line 623 also check the MCAlignFragment. Is it better to check MCAlignFragment at the beginning, and return without insert any MCBoundaryAlignFragment if the previous fragment is MCAlignFragment? In the following code, we can assume the previous fragment is not MCAlignFragment.
We shouldn't do that. Only NOP should not be emitted after a `MCAlignFragment`, since it the  `MCAlignFragment` is used to align the branch or the fused pair rather than NOP.  However, prefix can be emitted after a `MCAlignFragment`, since it is the part of the instruction.


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

https://reviews.llvm.org/D72225