From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com [IPv6:2607:f8b0:4864:20::22f]) by sourceware.org (Postfix) with ESMTPS id 18F57385840C for ; Fri, 23 Sep 2022 10:44:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 18F57385840C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-oi1-x22f.google.com with SMTP id c81so13744814oif.3 for ; Fri, 23 Sep 2022 03:44:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=Hgj0JP2QQScXM0uSyrCUE9CRg8RElwkC7NGcKmGgACQ=; b=OgOoJHh5WqbMCD064sgLii1v+yzjcxEuYGnGNDAIsm0ZRukRwCXjRas4cJt3bgJYN3 LX8eZoyxtdilJ1PEDBLZDykuAoBJAcIeCjC9jh8EL84BgpNsL80BjfY1+W+AH6MG8Rs9 Mu6Umwqi7ebRwDIwGgfrOeVGLu3l5+/74dFfjRDlkbQ6ei51rbmmc0vWHFF4MHAWjrcP uJ5WH13T2feZAiwucVEnMBi/oe27zZ/PolPBiGjtROowW34nhZBrmn3KUonRjAUxJLM9 vUs70dWj0VupT6waIQplQKVxqr2jn9ZjTH5YXZ48mCro0vZuNKgz6ixM3+pWOpvFI1D9 W3tg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=Hgj0JP2QQScXM0uSyrCUE9CRg8RElwkC7NGcKmGgACQ=; b=UjB65S1s2UWkPRrK2y4edaWwIp22HD5Wk3PawLgQS2srFEVvH/fRb4mMpuXdEb6Bmp jdkYpKm9oU1O3gpCK8s8svV61y5bMCwQkMAZ9+iqivD/CA/45f+52UQ7yZBwhidH/zRj ri4kXTqa/AySvw5Qc1VIpJZbiHrE97LauBY2I8fQarLVUOEWxAZJOmcnNf1GlGwznlYI J0lHPaPgUI8MWXFrvdwh5yEUL5qgNlmmSb3XhfOszqBfNNFVUDtWOyKZt13hLtEPETd1 76hKtpDED2bz6i4PTXHtCMD4BnwM4EoiE18IlcT8qGv4jzhdeTQzITGQ1zBUm4WhG0zr mWXQ== X-Gm-Message-State: ACrzQf2c9W6ccwEwMVayPIP00ts3uIb1G7HO4XOh2JnVSXyy1d47hk3D DOpts777lrIV++CoGgIUbDRjgVoCfI4xX79bkOFjNA== X-Google-Smtp-Source: AMsMyM65NaN0JhHpQ5jfya1LyNg/SSyYuEIMZrhKFvfG3zJtJKoeqaXfDa2xeKuTwtPHDyVpMq+9l4UvfOpKWNxiXOg= X-Received: by 2002:a05:6808:bca:b0:350:b22b:1283 with SMTP id o10-20020a0568080bca00b00350b22b1283mr8582569oik.82.1663929877448; Fri, 23 Sep 2022 03:44:37 -0700 (PDT) MIME-Version: 1.0 References: <20220923064726.9639-1-lifang_xia@linux.alibaba.com> <20220923094601.14338-1-lifang_xia@linux.alibaba.com> In-Reply-To: <20220923094601.14338-1-lifang_xia@linux.alibaba.com> From: Nelson Chu Date: Fri, 23 Sep 2022 18:44:26 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Optimize relax of GP with max_alignment. To: lifang_xia@linux.alibaba.com Cc: binutils@sourceware.org, gkm@rivosinc.com, Palmer Dabbelt Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Unfortunately, I got lots of gcc failures for rv64gc-elf regression, ========= Summary of gcc testsuite ========= | # of unexpected case / # of unique unexpected case | gcc | g++ | gfortran | rv64gc/ lp64d/ medlow |38068 / 4059 |20265 / 2738 | - | And I guess that is because we should pass "sec" rather than "sym_sec" to the _bfd_riscv_get_max_alignment_in_gp. Besides, not really sure if that may happen: do some sections that don't be considered in the range of gp in the first round, but will be considered in the later relax passes? If so, then does that make the max_alignment returned by the _bfd_riscv_get_max_alignment_in_gp have different values? Not really sure if that will break something. An idea is that - we probably can reserve the original max_alignment when determine if the section is in the range of gp, like this, https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-riscv.c#L4566. Though it seems a bit weird to estimate the alignment twice, but the original max alignment is used to estimate and try to cover most of the sections in the range of gp, and then we will get the new max alignment, which should be always smaller than the original one, and use it to estimate if the symbol is in the range if gp. However, I don't have any test case for now to approve what I am thinking, but that at least works for your testcase, too. Moreover, not sure if we can estimate the new max alignment for gp once also in here, https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-riscv.c#L4708. If we just use the original max alignment to estimate it, then doing once is probably enough. Happy to see any thoughts! Thanks Nelson On Fri, Sep 23, 2022 at 5:47 PM lifang_xia--- via Binutils wrote: > > From: Lifang Xia > > The max_alignment defined out of [gp-2K, gp+2k), the max_alignment > shouldn't affect the relax of gp. > If the symbol is in [gp-2K, gp+2k), the max_alignment would be > replaced with the max_alignment of the section in [gp-2k, gp+2k). > > bfd/ > * elfnn-riscv.c (_bfd_riscv_get_max_alignment_in_gp): New. > (_bfd_riscv_relax_lui): The max_alignment of sections is from > [gp-2K, gp+2K). > (_bfd_riscv_relax_pc): Likewise. > ld/ > * ld/testsuite/ld-riscv-elf/relax-max-align.*: New tests. > --- > bfd/elfnn-riscv.c | 37 +++++++++++++++++ > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 + > ld/testsuite/ld-riscv-elf/relax-max-align.d | 46 +++++++++++++++++++++ > ld/testsuite/ld-riscv-elf/relax-max-align.s | 29 +++++++++++++ > 4 files changed, 113 insertions(+) > create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align.d > create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align.s > > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c > index 0e0a0b09e24..83afc95ccab 100644 > --- a/bfd/elfnn-riscv.c > +++ b/bfd/elfnn-riscv.c > @@ -4259,6 +4259,29 @@ _bfd_riscv_get_max_alignment (asection *sec) > return (bfd_vma) 1 << max_alignment_power; > } > > +/* Traverse all output sections and return the max alignment > + in [gp-2K, gp+2K) . */ > + > +static bfd_vma > +_bfd_riscv_get_max_alignment_in_gp (asection *sec, bfd_vma gp) > +{ > + unsigned int max_alignment_power = 0; > + asection *o; > + > + if (sec == NULL) > + return 0; > + > + for (o = sec->output_section->owner->sections; o != NULL; o = o->next) > + { > + if ((VALID_ITYPE_IMM (sec_addr(o) - gp) > + || VALID_ITYPE_IMM (sec_addr(o) + o->size - gp)) > + && (o->alignment_power > max_alignment_power)) > + max_alignment_power = o->alignment_power; > + } > + > + return (bfd_vma) 1 << max_alignment_power; > +} > + > /* Relax non-PIC global variable references to GP-relative references. */ > > static bool > @@ -4290,6 +4313,13 @@ _bfd_riscv_relax_lui (bfd *abfd, > if (h->u.def.section->output_section == sym_sec->output_section > && sym_sec->output_section != bfd_abs_section_ptr) > max_alignment = (bfd_vma) 1 << sym_sec->output_section->alignment_power; > + else if (!undefined_weak) > + { > + /* Otherwise, consider the alignment of sections in [gp-2K,gp+2K). */ > + bfd_vma new_max_alignment = _bfd_riscv_get_max_alignment_in_gp (sym_sec, gp); > + if (new_max_alignment) > + max_alignment = new_max_alignment; > + } > } > > /* Is the reference in range of x0 or gp? > @@ -4556,6 +4586,13 @@ _bfd_riscv_relax_pc (bfd *abfd ATTRIBUTE_UNUSED, > if (h->u.def.section->output_section == sym_sec->output_section > && sym_sec->output_section != bfd_abs_section_ptr) > max_alignment = (bfd_vma) 1 << sym_sec->output_section->alignment_power; > + else if (!undefined_weak) > + { > + /* Otherwise, consider the alignment of sections in [gp-2K,gp+2K). */ > + bfd_vma new_max_alignment = _bfd_riscv_get_max_alignment_in_gp (sym_sec, gp); > + if (new_max_alignment) > + max_alignment = new_max_alignment; > + } > } > > /* Is the reference in range of x0 or gp? > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > index df89e0ee68b..e53f16facfa 100644 > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > @@ -169,6 +169,7 @@ if [istarget "riscv*-*-*"] { > run_dump_test "attr-merge-priv-spec-failed-05" > run_dump_test "attr-merge-priv-spec-failed-06" > run_dump_test "attr-phdr" > + run_dump_test "relax-max-align" > run_ld_link_tests [list \ > [list "Weak reference 32" "-T weakref.ld -m[riscv_choose_ilp32_emul]" "" \ > "-march=rv32i -mabi=ilp32" {weakref32.s} \ > diff --git a/ld/testsuite/ld-riscv-elf/relax-max-align.d b/ld/testsuite/ld-riscv-elf/relax-max-align.d > new file mode 100644 > index 00000000000..4ba920da970 > --- /dev/null > +++ b/ld/testsuite/ld-riscv-elf/relax-max-align.d > @@ -0,0 +1,46 @@ > +#source: relax-max-align.s > +#ld: > +#objdump: -d > + > +.*:[ ]+file format .* > + > + > +Disassembly of section .text: > + > +0+[0-9a-f]+ <_start>: > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+.* > +.*:[ ]+[0-9a-f]+[ ]+jal[ ]+.* > +.*:[ ]+[0-9a-f]+[ ]+j[ ]+.* > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > + > +0+[0-9a-f]+ : > +.*:[ ]+[0-9a-f]+[ ]+ret > +[ ]+... > diff --git a/ld/testsuite/ld-riscv-elf/relax-max-align.s b/ld/testsuite/ld-riscv-elf/relax-max-align.s > new file mode 100644 > index 00000000000..0d162ff4d93 > --- /dev/null > +++ b/ld/testsuite/ld-riscv-elf/relax-max-align.s > @@ -0,0 +1,29 @@ > + > +.global _start > +_start: > + lui a0, %hi(gdata) > + addi a0, a0, %lo(gdata) > + call func > + j . > + .size _start, . - _start > + > +.global func > +.align 7 > +func: > + ret > + .size func, . - func > + > +.data > +padding: > + .long 0 > + .long 0 > + .long 0 > + .long 0 > + .size padding, . - padding > + > +.global gdata > +.type gdata, object > +gdata: > + .zero 4 > + .size gdata, . - gdata > + > -- > 2.32.1 (Apple Git-133) >