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

Mon Mar 29 18:51:55 PDT 2021

Amir added a comment.

@MaskRay, @skan, appreciate your comments and suggestions!
@MaskRay:

> The patch should come with with tests.

Agree, still trying to find a way to test added functionality outside of BOLT's use case. Thank you for reviewing this patch and BOLT upstreaming repo!

@skan:

> Is the purpose of this patch to avoid cmp ends and jmp starts exactly at 64b alignment? If it's the only purpose, I think we can reuse the BoundaryAlign fragment by adding a function like "needPadding" in MCAssembler.cpp.

Yes, that's the purpose. We've looked at BoundaryAlign and aligned instruction bundles, but failed to come up with a simple solution. How do you envision BoundaryAlign avoiding the given alignment boundary falling between cmp and jmp? NeverAlign inserts a one-byte nop before cmp if it would end at a given boundary.

> Could you also upstream the code that inserts/remove the NeverAlign fragment and some test cases? Usage can help us determine if the design of the fragment is reasonable.

We seek to upstream LLVM parts required by BOLT before pushing BOLT. 
The usage is shown here: https://github.com/facebookincubator/BOLT/blob/68abc968b706b55585b1b8be315aef5d3bf90b1c/bolt/src/BinaryEmitter.cpp#L445
We analyze the basic block for macro-op fusion pairs; if such a pair is found, we insert NeverAlign fragment after the first instruction (cmp) with a call `MCStreamer::emitNeverAlignCodeAtEnd()`. That's the only external use of the interfaces we add, which makes it non-trivial to test. That's why I'm seeking to add a way to trigger the insertion of NeverAlign fragment (see comment above).

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

https://reviews.llvm.org/D97982