From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out30-43.freemail.mail.aliyun.com (out30-43.freemail.mail.aliyun.com [115.124.30.43]) by sourceware.org (Postfix) with ESMTPS id AC4E13858D1E for ; Sat, 24 Sep 2022 01:54:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AC4E13858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.alibaba.com X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R381e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018046059;MF=lifang_xia@linux.alibaba.com;NM=1;PH=DS;RN=4;SR=0;TI=SMTPD_---0VQYbXa._1663984484; Received: from 192.168.3.7(mailfrom:lifang_xia@linux.alibaba.com fp:SMTPD_---0VQYbXa._1663984484) by smtp.aliyun-inc.com; Sat, 24 Sep 2022 09:54:45 +0800 Message-ID: Date: Sat, 24 Sep 2022 09:54:44 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 Subject: Re: [PATCH] RISC-V: Optimize relax of GP with max_alignment. To: Nelson Chu Cc: binutils@sourceware.org, gkm@rivosinc.com, Palmer Dabbelt References: <20220923064726.9639-1-lifang_xia@linux.alibaba.com> <20220923094601.14338-1-lifang_xia@linux.alibaba.com> From: Lifang Xia In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-20.8 required=5.0 tests=BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,UNPARSEABLE_RELAY,USER_IN_DEF_SPF_WL 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: On 2022/9/23 18:44, Nelson Chu wrote: > 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. Agree with that. > > 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. It would be occurence if the gp is assigned with a absolute address, such as "__global_point = 0x4000". > 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. That's is conservative idea, it would work fine for both of the test cases. > 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. It is not a good choice. Because the max alignment is only used for gp, but not all of the relax need it. Thanks, Lifang > > 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) >>