[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
Tue Aug 13 04:23:34 PDT 2019


MaskRay added a subscriber: atanasyan.
MaskRay added a comment.

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


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