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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 20 17:01:31 PST 2019


MaskRay added a comment.

In D70157#1793280 <https://reviews.llvm.org/D70157#1793280>, @reames wrote:

> I've gone ahead and landed the patch so that we can iterate in tree.  See commit 14fc20ca62821b5f85582bf76a467d412248c248 <https://reviews.llvm.org/rG14fc20ca62821b5f85582bf76a467d412248c248>.
>
> I've also landed a couple of follow up patches to address issues which would have otherwise required iteration on the review.  See commits c148e2e2ef86f53391be459752511684424f331b <https://reviews.llvm.org/rGc148e2e2ef86f53391be459752511684424f331b>, 4024d49edc1598a6f8017df541147b38bf1e2818 <https://reviews.llvm.org/rG4024d49edc1598a6f8017df541147b38bf1e2818>, and 8b725f0459eee468ed7f9935fba3278fcb4997b1 <https://reviews.llvm.org/rG8b725f0459eee468ed7f9935fba3278fcb4997b1>.
>
> I still see some room for further cleanup (i.e. the fragment range scheme and tests), but what's in is of reasonable quality.
>
> There's a couple follow up patches which are probably called for, but I think we can work on these in parallel now.
>
> 1. We need to settle on assembler syntax.
> 2. We need a patch for the x86 MI to MC translation to mark regions unsafe to pad.  (Probably best to separate from the above for the moment.)
> 3. We can incrementally add support for prefix padding.


If you planned to clean up, you could have made the cleanups in the original land:) Though there would be a problem of proper contribution attribution. After Subversion->Git transition, we can actually run `git commit --amend --author=ask-author-to-provide-name-and- <email>' (Though changing the author may not be fair to your contribution to this commit... oh it is so difficult.) Anyway, it is nice to see this feature before Christmas and people can start investigating its impact now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157





More information about the llvm-commits mailing list