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

Eric Christopher via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 14 16:39:52 PST 2021


On Tue, Dec 14, 2021 at 4:12 PM Fangrui Song via Phabricator <
reviews at reviews.llvm.org> wrote:

> MaskRay added inline comments.
>
>
> ================
> Comment at: lld/test/ELF/aarch64-adrp-ldr-got.s:97
> +.global main
> +main:
> +  adrp    x1, :got:x
> ----------------
> echristo wrote:
> > alexander-shaposhnikov wrote:
> > > MaskRay wrote:
> > > > alexander-shaposhnikov wrote:
> > > > > MaskRay wrote:
> > > > > > Unlike Mach-O, ELF uses `_start`. Missing `_start` leads to a
> warning in an executable link.
> > > > > >
> > > > > > If you combine the tests, lots of boilerplate (duplicate
> `.rodata`, duplicate `main:`, etc) can be avoided.
> > > > > one day the tests will fail and debugging a small input is much,
> much easier.
> > > > > I'd prefer to keep them separate.
> > > > I may disagree with this. AArch64 does not decrease the number of
> instructions. If FileCheck reports an error at one line, it is quick to
> know associated the error location with the source, especially if you use
> distinct symbols in the first place.
> > > I'm sorry, but I have to disagree. It's not the problem with
> FileCheck, a lot of things are happening inside the linker,
> > > it's easier to reason about (and to debug) small independent inputs
> rather than bigger binaries (especially when multiple different code paths
> are triggered intentionally).
> > FWIW smaller tests in separate files also parallelize more easily during
> test runs if it helps. Hopefully the boiler plate won't need to be adjusted
> often or do you have a different concern?
> >
> I agree that small independent inputs are sometimes easier to reason
> about, but I don't think combining good-to-relax tests reduce readability.
> If you combine, logically the code blocks are still isolated. If you want
> to make them more isolated, you can add labels `test_hidden: adrp    x1,
> :got:hidden` and use CHECK-LABEL (
> https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-label-directive
> ).
>
> The RUN invocations are in one file. They don't parallelize in the lit
> runner.
> Combining can cut down a large amount of boilerplate, and IMHO improve
> readability for the source file and the RUN lines. Every relocatable object
> file and output incur some recognition cost as well.
>

I may have misunderstood the combined versus separate tests. I'd probably
have separate tests in separate files, but do agree that if they're all in
one file you may as well save some boilerplate :)

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211214/abbf7c9e/attachment.html>


More information about the llvm-commits mailing list