[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

Fri Dec 20 00:47:07 PST 2019

MaskRay added a comment.

In D70157#1792262 <https://reviews.llvm.org/D70157#1792262>, @skan wrote:

> Do you think this patch is ready to land? @MaskRay

It is 00:35 here and I should confess that I haven't read through this yet.

There are some minor things like (1) pervasive `auto` (2) `report_fatal_error` in `X86AlignBranchKind` is not suitable. I expect the error is reported at a command line parsing stage. (3) I don't see how `__tls_get_addr` is referenced in code but somehow it magically appears in tests. It may be related to `VK_NONE` but there should be more tests for `!VK_NONE` cases. (4) More function-level comments are needed.

If it is not super urgent, probably want a couple of hours to get a response from @reames or @jyknight whether they think this can be committed as is, with iteration work in the future?

(Also note that @fedor.sergeev has requested changes. The convention is to wait for @fedor.sergeev to agree that this can be accepted... but there are vacation schedules involved.)

================
Comment at: llvm/lib/MC/MCAssembler.cpp:1009
+  AlignedOffset -= OldSize;
+  auto BoundaryAlignment = BF.getAlignment();
+  uint64_t NewSize = needPadding(AlignedOffset, AlignedSize, BoundaryAlignment)
----------------
`Align`

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

https://reviews.llvm.org/D70157