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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
