From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x236.google.com (mail-oi1-x236.google.com [IPv6:2607:f8b0:4864:20::236]) by sourceware.org (Postfix) with ESMTPS id 991B53858C2D for ; Tue, 27 Sep 2022 08:51:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 991B53858C2D 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-x236.google.com with SMTP id c81so11163444oif.3 for ; Tue, 27 Sep 2022 01:51:11 -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=c8qzoRtn1X5kKCUsMmI5qlbPCo1ZkaRb+NLnXxJO1d8=; b=gSOAPYQ2CgPetPKHVE15aeRRsM5/HNhdIuhYplGgc/cFXtDUWeTpUfErDvCWIT1i48 8D38g1JqBNWuYBN4gun/zuRMcwi+13ol3IpAzOIEo4GaMN4V3uT34TBNWDRMqtOZJo0I 5oyGUJvrDh43/+Bkq91nkQdEV6UPMYsfI+a85Qv0dseeigGBLgxu4reEDebcJJ0eaMG5 AeF9LyUOamVCM9dF2vtFRh3tUp1Tb+sYnCZkIeBf8jEfMwy6Rlj6XHOTBHUiKFmviS/a 91keBXkhoX7B50hspEillo5ZNNDyyLFqS0X8zfPxrP6thuYJBQCdgBeL5ZpfgxOfQ9Vj QFAA== 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=c8qzoRtn1X5kKCUsMmI5qlbPCo1ZkaRb+NLnXxJO1d8=; b=2h7odhDq6+JOF0EX3QBdp7NLCyRAz08k4eQXHNhp32doxYglXT2bKci8aNuO3nvrSZ A5arkr18UgAlSx5Ocg0Uil7tFcQei4tsCyhwjsiwPtLdSosEQnid/adbEKrGg/hG02LT hT6BpaabeA/oTZ/hK6+6m1iXPzhorSCzMayYGcHjNtlIi0jX88jVeorraznpugLvhAXW YJntNXXnD49d/Nu8GiTdKwggdUODReV1x+RU57MSe8u5L6eXVO7qJURZvIf6SFA3M/hq wh8PUzb3dhP6BTKoReUF4kuQg2ft8SmsJKFdmeiQDg3v0llTVKhp9iW9I3XNekTvsIvD XQ3Q== X-Gm-Message-State: ACrzQf0JUs1v83/ESud3SsRU/1btCKTzhgoBSQ8OGVIzP6f0MOw6yf9B az4VQ3wl1tJjm6tTUJH7XIgnEm2Sbq3GecdmZ07tFw== X-Google-Smtp-Source: AMsMyM4R1PWibgCYBCdMJVRq42P0VL42tdz/ZjtY69uH11LM3OlPUGqRLZY3OF632jmevWgGIzeYi34amZ0Wsc+BsyM= X-Received: by 2002:a05:6808:bca:b0:350:b22b:1283 with SMTP id o10-20020a0568080bca00b00350b22b1283mr1288844oik.82.1664268670746; Tue, 27 Sep 2022 01:51:10 -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: From: Nelson Chu Date: Tue, 27 Sep 2022 16:51:00 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Optimize relax of GP with max_alignment. To: Lifang Xia Cc: binutils@sourceware.org, gkm@rivosinc.com, Palmer Dabbelt Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.1 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: On Sat, Sep 24, 2022 at 9:54 AM Lifang Xia wrote: > > > 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, make sense. Then is it possible to estimate the max alignment for gp only once in the _bfd_riscv_get_max_alignment_in_gp for each relax_pass? Nelson > 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) > >>