[PATCH] D112063: [lld][ELF] Add first bits to support relocation relaxations for AArch64

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/AArch64.cpp:648
+
+  Relocation adrpSymRel = {
+      R_AARCH64_PAGE_PC,          // expr
----------------
alexander-shaposhnikov wrote:
> MaskRay wrote:
> > In other replaces we just use the concise form to initialization a `Relocation`
> > 
> > ```
> >    Relocation adrpRel{R_AARCH64_PAGE_PC, R_AARCH64_ADR_PREL_PG_HI21,
> >                       adrpRel.offset, 0, &sym};
> > ```
> > I assume that `adrpSymRel` can be named to `adrpRel`
> honestly I find the current version easier to read than the concise one (e.g.  what does 0 mean there ?  one has to look it up and memorize) 
Then just comment this specific field: ``/*addend=*/0`. Other arguments hint the field names.


================
Comment at: lld/test/ELF/aarch64-adrp-ldr-got.s:27
+## Case 3. LDR and ADRP use different registers, no relaxations should be applied.
+# RUN: ld.lld %t-mismatched-src-reg.o -o %t-mismatched-src-reg.exe
+# RUN: llvm-objdump --no-show-raw-insn -d %t-mismatched-src-reg.exe \
----------------
alexander-shaposhnikov wrote:
> MaskRay wrote:
> > For executables, just omit the suffix name. Avoid `.exe`
> I'm curious - is this documented anywhere or is it the local code style adopted by LLD? 
> having .exe is convenient since it gives a hint to the reader / or the person who runs the test that this particular file is an executable (or at least intended to be)  - grepping the LLVM repository returns a huge number of places (tests) where this is used.  I do not mind dropping it here - but really curious about the rationale for it.
Omitting `.exe` is the prevailing style in test/ELF. A relocatable object file is named `%t.o` and the executable is `%t`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112063