[PATCH] D101817: [MC][X86] Automatic alignment for Macro-Op Fusion

[Kan Shengchen via Phabricator via llvm-commits]

skan added inline comments.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:165-172
+    if (X86AlignForMacroFusion) {
+      assert(!X86AlignBranchWithin32BBoundaries &&
+             "X86AlignForMacroFusion is incompatible with "
+             "X86AlignBranchWithin32BBoundaries");
+      AlignBoundary = assumeAligned(64);
+      AlignBranchType.addKind(X86::AlignBranchFused);
+      AlignBranchType.addKind(X86::AlignBranchJcc);
----------------
Amir wrote:
> skan wrote:
> > In fact, X86AlignForMacroFusion is compatible with X86AlignBranchWithin32BBoundaries, you can set AlignBoundary to 64 and add  AlignBranchFused to AlignBranchType if both of the options exist.
> No, they're semantically incompatible: X86AlignBranchWithin32BBoundaries would prevent macrofusion pair from crossing the 32B boundary (stronger alignment restriction), while X86AlignForMacroFusion would prevent macrofusion pair being perfectly split by 64B boundary.
> So if X86AlignForMacroFusion is on, it's known not to satisfy X86AlignBranchWithin32BBoundaries restrictions. 
> (Conversely, if X86AlignBranchWithin32BBoundaries is on, X86AlignForMacroFusion is satisfied automatically).
> 
> They're further incompatible by the fact that X86AlignForMacroFusion flag enables the new BoundaryAlign behavior `isAvoidEndAlign` which performs the same alignment as NeverAlign.
"So if X86AlignForMacroFusion is on, it's known not to satisfy X86AlignBranchWithin32BBoundaries restrictions.
(Conversely, if X86AlignBranchWithin32BBoundaries is on, X86AlignForMacroFusion is satisfied automatically)."

The requirements of X86AlignForMacroFusion and X86AlignBranchWithin32BBoundaries can be met at the same time, so they are compatible.

`isAvoidEndAlign`  is a detail about implementation and is not a good proof for the incompatibility.

Meanwhile, AlignBranchType, AlignBoundary can be also be set by options X86AlignBranchBoundary, X86AlignBranch. So the assert here is not correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101817