[PATCH] D75438: [X86] Reduce the number of emitted fragments due to branch align

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:522
+
+  if (!isMacroFused(PrevInst, Inst))
+    // Macro fusion doesn't happen indeed, clear the pending.
----------------
PendingBoundaryAlign has been set to nullptr in alignBranchesEnd()?



================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:565
+
+  if (!needAlignInst(Inst) || !PendingBoundaryAlign)
+    return;
----------------
Only check PendingBoundaryAlign is enough?  In alignBranchesBegin(), if needAlignInst(Inst), a PendingBoundaryAlign is added.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:570
+  PendingBoundaryAlign->setLastFragment(OS.getCurrentFragment());
+  PendingBoundaryAlign = nullptr;
+
----------------
After setting PendingBoundaryAlign to nullptr, it seems there is no chance to reset LastFragment of PendingBoundaryAlign for macro fusion case.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:572
+
+  // We need to ensure that further data isn't added to the current data
+  // fragment. The easiest way is to insert a new empty DataFragment.
----------------
I don't understand why further data isn't added to the current data. Is it because we need get instruction size? Could you comments it in the code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75438