[PATCH] D64930: [ELF][AArch64] Allow PT_LOAD to have overlapping p_offset ranges

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 19 23:11:37 PDT 2019


MaskRay added a comment.

In D64930#1626816 <https://reviews.llvm.org/D64930#1626816>, @MaskRay wrote:

> In D64930#1626651 <https://reviews.llvm.org/D64930#1626651>, @peter.smith wrote:
>
> > I'm personally in favour of accepting D64930 <https://reviews.llvm.org/D64930> and D64906 <https://reviews.llvm.org/D64906> (I think that there is also D65865 <https://reviews.llvm.org/D65865> on X86) as I think that this is an important size optimization already performed by Gold and BFD. Thinking about how to progress this further, I think it would be good to write a summary of where each of these reviews is at and what the plan is for this feature. For example:
>
>
> Thanks for pushing forward this patch series!
>
> > - Have all the comments been addressed, I think the main concern was about TLS?
>
> All addressed. There was concern about TLS but I aligned p_vaddr/p_offset to p_align of PT_TLS.
>
> > - Which targets do we want this to apply to, all of them, or just those with a 64k page size?
>
> All, probably except x86_64 (with max-page-size=4096).
>
> `bool enable = config->emachine != EM_X86_64 || config->maxPageSize != 4096;`
>
> > - What is the dependency tree of the reviews, I think D64906 <https://reviews.llvm.org/D64906> is a pre-requisite for all of them?
>
> A tree with a single internal node:) Yes, D64906 <https://reviews.llvm.org/D64906> is a pre-requisite for all of them.
>
>   D64906 base, PPC64
>     D64930 AArch64 47 tests
>     D65865 i386 33 tests
>     Dxxxxx ARM (30~50 tests)
>     Dxxxxx MIPS
>     rLLDxxxxx RISC-V <10 tests
>     rLLD PPC32 <10 tests
>     Dxxxxx x86_64 with maxpagesize != 4096
>
>
> Each new target can be enabled independently. The code change is trivial:
>
>   -      bool enable = config->emachine == EM_PPC64;
>   +      bool enable =
>   +          config->emachine == EM_AARCH64 || config->emachine == EM_PPC64;
>
>
> The only problem is the number of the affected tests. Due to the layout change nature of the patch series, lots of addresses have to be updated. x86_64 with maxpagesize=4096 is estimated to affect 300+ tests.
>
> > - Does anyone have any objection, or are they happy for individual people active in a backend to approve that part?
>
> We haven't heard from MIPS yet. @atanasyan


@ruiu Let's move D64906 <https://reviews.llvm.org/D64906> and D64930 <https://reviews.llvm.org/D64930> forward?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D64930





More information about the llvm-commits mailing list