[PATCH] D122065: [BOLT] Align constant islands to 8 bytes

Maksim Panchenko via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 24 13:12:55 PDT 2022


maksfb added a comment.

In D122065#3406049 <https://reviews.llvm.org/D122065#3406049>, @yota9 wrote:

> I don't really mind, but sometimes it is just inconvenient  to make such tests. And I still would like some tests to be runnable.. I think we need to discuss the testing policy.

It's fair to say that sometimes creating non-runnable test is way easier. However, in this case, will it be sufficient to locate the island in the output binary (e.g. with `llvm-objdump`) and make sure the address is aligned (`{{0|8}}`)?

>> Regarding the change itself, have you considered aligning constant islands so that they straddle a minimum number of cache lines?
>
> This could be discussed, but we need to check if there will be any winning in this, I don't think this is a good way just to align to cache line size, it should be more clever system, but this is beyond the goals of this review. But thank you for you suggestion anyway :)

Totally, I don't mean it for this diff. However, could you create a constant for the alignment (instead of `sizeof(uint64_t)`) that could be easily changed for running experiments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122065



More information about the llvm-commits mailing list