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

[Amir Ayupov via Phabricator via llvm-commits]

Amir added a comment.

@lebedev.ri:

> Where is this fragment going to be added? The alignment will differ per CPU.

This fragment is inserted before first instruction in macro-fusion pair (cmp instruction in cmp+jcc pair). Modern Intel Cores have a macro-fusion restriction that cmp+jcc shouldn't be split by a cache line boundary. It's OK to for cmp instruction to cross cache line boundary. Not all X86 cores have macro-fusion or this restriction, so the insertion policy and alignment is up to the MC client (BOLT or assembly programmer).

> Do fragments only have the start point, or can they specify a range of instructions? If they can, then NeverAlign seems contrived to me. Perhaps this should instead be generalized into something like "ensure that this group of instructions doesn't cross ?-byte alignment"?

This fragment only looks at the subsequent fragment (instruction) in the stream. It doesn't make sense for us to generalize to a range of instructions. There's BoundaryAlign fragment type that can handle alignment boundary crossing for macro-fusion pairs for Intel JCC erratum mitigation, or BundleAlign fragment that can align a group of instructions to the specified alignment (left, right alignment inside bundle, introduced by PNaCl to ensure control-flow integrity). These fragments are more general than NeverAlign, and come with a higher overhead for the client.



================
Comment at: llvm/include/llvm/MC/MCObjectStreamer.h:142
                          unsigned MaxBytesToEmit = 0) override;
+  void emitNeverAlignCodeAtEnd(unsigned ByteAlignment) override;
   void emitValueToOffset(const MCExpr *Offset, unsigned char Value,
----------------
MaskRay wrote:
> Why "AtEnd"?
"End" is the end of the subsequent fragment that we want _not_ be at a given boundary. 
Can be replaced with (arguably) more descriptive like `emitAvoidEndAlign`, similar to directive name. What do you think?


================
Comment at: llvm/include/llvm/MC/MCStreamer.h:838
+  /// If the end of the following fragment ever gets aligned to
+  /// \p ByteAlignment, emit a single nop to break this alignment.
+  virtual void emitNeverAlignCodeAtEnd(unsigned ByteAlignment);
----------------
lebedev.ri wrote:
> emit *where*?
Thanks for flagging! The wording might be a bit misleading. It'd be more clear to state it as "If the end of the fragment following this NeverAlign fragment ever gets aligned to \p ByteAlignment, this fragment emits a single nop before the following fragment to break this end-alignment."



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

https://reviews.llvm.org/D97982