From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from xry111.site (xry111.site [89.208.246.23]) by sourceware.org (Postfix) with ESMTPS id 567B73858C52 for ; Thu, 5 Oct 2023 10:09:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 567B73858C52 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=xry111.site Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=xry111.site DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=xry111.site; s=default; t=1696500563; bh=0W2z9J6kK/U2xyc9o/gDbNZGn1h2ETSBeisvjwnlNrw=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=Td1BJ5b2wUgsJryZ5dQfUf+JN01MYPqDOOL+eXVRmlwv10BTEI2DqZ23BbmB5KKLI stKiPT90dsk9nsW4h+ZubBkqGGmVrGCdaequxA0Pk0NdwhtaHKlRaGrYs7ValZY09+ 69cU1y0i0nqtrsfmZ7LBb/AgqXVYCpk/dOXAinKQ= Received: from [127.0.0.1] (unknown [IPv6:2001:470:683e::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature ECDSA (P-384) server-digest SHA384) (Client did not present a certificate) (Authenticated sender: xry111@xry111.site) by xry111.site (Postfix) with ESMTPSA id 02A5D669C3; Thu, 5 Oct 2023 06:09:21 -0400 (EDT) Message-ID: <16094953d52e88d269591129ed9aed3efa6ae761.camel@xry111.site> Subject: Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section From: Xi Ruoyao To: Jinyang He , mengqinggang , Chenghua Xu , Zhensong Liu , WANG Xuerui Cc: binutils@sourceware.org, Xing Li , yala , Peng Fan Date: Thu, 05 Oct 2023 18:09:20 +0800 In-Reply-To: References: <20230711084931.18978-1-hejinyang@loongson.cn> <679cafe2-f5fb-dc76-61db-f5ecb6fd1776@loongson.cn> <98e6ae12-404d-010a-4640-b116b4c6899b@loongson.cn> <2b547680-2a07-b3bd-933a-955910981bc2@loongson.cn> Autocrypt: addr=xry111@xry111.site; prefer-encrypt=mutual; keydata=mDMEYnkdPhYJKwYBBAHaRw8BAQdAsY+HvJs3EVKpwIu2gN89cQT/pnrbQtlvd6Yfq7egugi0HlhpIFJ1b3lhbyA8eHJ5MTExQHhyeTExMS5zaXRlPoiTBBMWCgA7FiEEkdD1djAfkk197dzorKrSDhnnEOMFAmJ5HT4CGwMFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4AACgkQrKrSDhnnEOPHFgD8D9vUToTd1MF5bng9uPJq5y3DfpcxDp+LD3joA3U2TmwA/jZtN9xLH7CGDHeClKZK/ZYELotWfJsqRcthOIGjsdAPuDgEYnkdPhIKKwYBBAGXVQEFAQEHQG+HnNiPZseiBkzYBHwq/nN638o0NPwgYwH70wlKMZhRAwEIB4h4BBgWCgAgFiEEkdD1djAfkk197dzorKrSDhnnEOMFAmJ5HT4CGwwACgkQrKrSDhnnEOPjXgD/euD64cxwqDIqckUaisT3VCst11RcnO5iRHm6meNIwj0BALLmWplyi7beKrOlqKfuZtCLbiAPywGfCNg8LOTt4iMD Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.50.0 MIME-Version: 1.0 X-Spam-Status: No, score=-6.1 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,LIKELY_SPAM_FROM,SPF_HELO_PASS,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: Hi Jinyang, Any progress on this? During my attempt trying to balance relaxation and scheduling better in GCC I found the elf_seg_map condition is preventing relaxation on *every* shared library (when we are linking a shared library the elf_seg_map of .text is NULL). On a modern system most code paths are in shared libraries, so it's really bad not to perform the relaxation on them. Esp. now we are disabling explicit relocs for relaxation, so if we don't relax shared libraries we are likely regressing the overall performance of the system. Sorry for disturbing you during the national holiday, feel free to defer the reply until the holiday ends! On Thu, 2023-07-13 at 16:18 +0800, Jinyang He wrote: > On 2023-07-13 15:19, Jinyang He wrote: >=20 > > Hi, > >=20 > >=20 > > We should pay attention to them, the bug in relaxation. Relaxation may= =20 > > not work correctly because of the bug1 reported by the second patch=20 > > and the bug2 reported in the my previous email, although it works well= =20 > > in most of the time. Those 2 bugs are just what we've found, but there= =20 > > may be hidden problems. > > I also tested the bug2 on RISCV and although it couldn't be resolved,= =20 > > it reported an error. > >=20 > > Test is, > > $ cat c.s > > =C2=A0 .section ".another.text", "x" > > =C2=A0 .fill 1, 4, 0 > > =C2=A0 call fn2 > > =C2=A0 call fn2 > > fn1: > > =C2=A0 nop > >=20 > > .text > > fn2: > > =C2=A0 .fill 4, 4, 0 > > =C2=A0 call fn1 > >=20 > > $ ./gas/as-new=C2=A0 -march=3Drv32ic_zicsr -mno-arch-attr=C2=A0 -o c.o = c.s > > $ ./ld/ld-new c.o -o c -e 0 --section-start=3D.another.text=3D0x1000000= =20 > > -Ttext 0x1100000 > > c.o: in function `fn2': > > (.text+0x10): relocation truncated to fit: R_RISCV_JAL against `fn1' > >=20 > > So, I'll find the reason why reloc_bits_pcrel20_s2() not reports the > > overfollow error, and add error report which I think it is more=20 > > important than these two patches so that we can get feedback when we > > link. > >=20 >=20 > Now, I find that the sign-overflow-check has bug, include=20 > reloc_bits_pcrel20_s2(), reloc_bits_b26() and (others, I did not test=20 > them). And I think the "mask" should be > =C2=A0=C2=A0=C2=A0=C2=A0 bfd_signed_vma mask =3D ((bfd_signed_vma)0x1 << = (howto->bitsize -=20 > (sign ? 1 : 0)) - 1; >=20 > The test about reloc_bits_b26() is, >=20 > $ cat d.s > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .section ".another.text"= , "x" > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 b=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 sym1 >=20 > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .text > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 nop > sym1: > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 nop >=20 > $ ./gas/as-new -mabi=3Dlp64d -o d.o d.s > $ ./ld/ld-new d.o -o d -e 0 --section-start=3D.another.text=3D0x130000000= =20 > -Ttext 0x120000000 >=20 > I'll do fix and send v2 after you test spec2006. Or other responses? >=20 >=20 > Thanks! >=20 >=20 > >=20 > > On 2023-07-13 11:13, mengqinggang wrote: > > > Hi, > > >=20 > > > I will apply this two patches to see if there are any errors in=20 > > > specc2006. > > >=20 > > > =E5=9C=A8 2023/7/12 =E4=B8=8B=E5=8D=884:08, Jinyang He =E5=86=99=E9= =81=93: > > > > Hi, > > > >=20 > > > >=20 > > > > I'm not fimiliar with the process of output binary. I just noted > > > > that it calls lang_relax_sections() before calling=20 > > > > _bfd_elf_map_sections_to_segments() in ldelf_map_segments(). Thus I= =20 > > > > think the condition, elf_seg_map (info->output_bfd) =3D=3D NULL, al= ways=20 > > > > be true when first time it calls loongarch_elf_relax_section().=20 > > > > Actually we can get right symbol value when relaxation, but ... > > > >=20 > > > > You reminder me that the symbol value will be reduced due to=20 > > > > relaxation, which like the instruction pc is reduced. Then we canno= t=20 > > > > get right symbol value if the symbol in relaxable section. And I > > > > write a test, > > > >=20 > > > > $ cat c.s > > > > =C2=A0=C2=A0=C2=A0 .section ".another.text", "x" > > > > =C2=A0=C2=A0=C2=A0 nop > > > > =C2=A0=C2=A0=C2=A0 la.local $a0, sym1 > > > > sym2: > > > > =C2=A0=C2=A0=C2=A0 nop > > > >=20 > > > > =C2=A0=C2=A0=C2=A0 .text > > > > sym1: > > > > =C2=A0=C2=A0=C2=A0 nop > > > > =C2=A0=C2=A0=C2=A0 nop > > > > =C2=A0=C2=A0=C2=A0 nop > > > > =C2=A0=C2=A0=C2=A0 la.local $a0, sym2 > > > >=20 > > > > $ ./gas/as-new -mabi=3Dlp64d -o c.o c.s > > > > $ ./ld/ld-new c.o -o c -e 0=20 > > > > --section-start=3D.another.text=3D0x120000000 -Ttext 0x120200000 > > > >=20 > > > > Disassembly of section .another.text: > > > >=20 > > > > 0000000120000000 : > > > > =C2=A0=C2=A0 120000000:=C2=A0=C2=A0=C2=A0 03400000 =C2=A0=C2=A0=C2= =A0 andi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 $zero= , $zero, 0x0 > > > > =C2=A0=C2=A0 120000004:=C2=A0=C2=A0=C2=A0 18ffffe4 =C2=A0=C2=A0=C2= =A0 pcaddi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 $a0, 524287(0x7= ffff) > > > >=20 > > > > 0000000120000008 : > > > > =C2=A0=C2=A0 120000008:=C2=A0=C2=A0=C2=A0 03400000 =C2=A0=C2=A0=C2= =A0 andi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 $zero= , $zero, 0x0 > > > >=20 > > > > Disassembly of section .text: > > > >=20 > > > > 0000000120200000 : > > > > =C2=A0=C2=A0 120200000:=C2=A0=C2=A0=C2=A0 03400000 =C2=A0=C2=A0=C2= =A0 andi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 $zero= , $zero, 0x0 > > > > =C2=A0=C2=A0 120200004:=C2=A0=C2=A0=C2=A0 03400000 =C2=A0=C2=A0=C2= =A0 andi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 $zero= , $zero, 0x0 > > > > =C2=A0=C2=A0 120200008:=C2=A0=C2=A0=C2=A0 03400000 =C2=A0=C2=A0=C2= =A0 andi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 $zero= , $zero, 0x0 > > > > =C2=A0=C2=A0 12020000c:=C2=A0=C2=A0=C2=A0 18ffffe4 =C2=A0=C2=A0=C2= =A0 pcaddi=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 $a0, 524287(0x7= ffff) > > > >=20 > > > > Due to the bug repaired by Patch2 and the bug reported by this test= .=20 > > > > And I also did not see that condition in other archs. I'm not sure= =20 > > > > that the relaxation spec2006 error is caused by that condition or= =20 > > > > those bugs. > > > >=20 > > > >=20 > > > > On 2023-07-12 14:35, mengqinggang wrote: > > > > > Hi, > > > > >=20 > > > > > Without this condition, the value of 'symbol - pc' may be=20 > > > > > differences between > > > > > loongarch_elf_relax_section and loongarch_elf_relocate_section. > > > > > And it will cause some spec2006 test errors. > > > > >=20 > > > > > =E5=9C=A8 2023/7/11 =E4=B8=8B=E5=8D=884:49, Jinyang He =E5=86=99= =E9=81=93: > > > > > > When I tried "./ld-new sth." the relax does not work because th= e > > > > > > elf_seg_map (info->output_bfd) was NULL in=20 > > > > > > loongarch_elf_relax_section. > > > > > > There is no need for the segment info, we can get the address t= hrough > > > > > > the section info. Thus, remove this condition. > > > > > >=20 > > > > > > bfd/ChangeLog: > > > > > >=20 > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0* elfnn-loongarch.c (loongarch_elf_rela= x_section): Remove > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0elf_seg_map condition. > > > > > >=20 > > > > > > ld/ChangeLog: > > > > > >=20 > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0* testsuite/ld-loongarch-elf/relax.exp:= Update trigger method. > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0* testsuite/ld-loongarch-elf/relax.dd: = New test. > > > > > > --- > > > > > > =C2=A0 bfd/elfnn-loongarch.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= |=C2=A0 1 - > > > > > > =C2=A0 ld/testsuite/ld-loongarch-elf/relax.dd=C2=A0 |=C2=A0 6 += ++++ > > > > > > =C2=A0 ld/testsuite/ld-loongarch-elf/relax.exp | 35=20 > > > > > > +++++++------------------ > > > > > > =C2=A0 3 files changed, 16 insertions(+), 26 deletions(-) > > > > > > =C2=A0 create mode 100644 ld/testsuite/ld-loongarch-elf/relax.d= d > > > > > >=20 > > > > > > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c > > > > > > index d3d8419d8..01f349a24 100644 > > > > > > --- a/bfd/elfnn-loongarch.c > > > > > > +++ b/bfd/elfnn-loongarch.c > > > > > > @@ -3845,7 +3845,6 @@ loongarch_elf_relax_section (bfd *abfd,= =20 > > > > > > asection *sec, > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 || sec->sec_flg0 > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 || (sec->flags & SEC= _RELOC) =3D=3D 0 > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 || sec->reloc_count = =3D=3D 0 > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 || elf_seg_map (info->output_bf= d) =3D=3D NULL > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 || (info->disable_ta= rget_specific_optimizations > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 && info->relax_pass = =3D=3D 0) > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* The exp_seg_relro= _adjust is enum phase_enum (0x4), > > > > > > diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd=20 > > > > > > b/ld/testsuite/ld-loongarch-elf/relax.dd > > > > > > new file mode 100644 > > > > > > index 000000000..e266beb8f > > > > > > --- /dev/null > > > > > > +++ b/ld/testsuite/ld-loongarch-elf/relax.dd > > > > > > @@ -0,0 +1,6 @@ > > > > > > +#... > > > > > > +.*pcaddi.* > > > > > > +.*ld.w.* > > > > > > +.*pcaddi.* > > > > > > +.*ld.w.* > > > > > > +#pass > > > > > > diff --git a/ld/testsuite/ld-loongarch-elf/relax.exp=20 > > > > > > b/ld/testsuite/ld-loongarch-elf/relax.exp > > > > > > index 7ff876d79..d2bb967c6 100644 > > > > > > --- a/ld/testsuite/ld-loongarch-elf/relax.exp > > > > > > +++ b/ld/testsuite/ld-loongarch-elf/relax.exp > > > > > > @@ -21,31 +21,6 @@ > > > > > > =C2=A0 =C2=A0 if [istarget loongarch64-*-*] { > > > > > > =C2=A0 -=C2=A0 if [isbuild loongarch64-*-*] { > > > > > > -=C2=A0=C2=A0=C2=A0 set testname "loongarch relax build" > > > > > > -=C2=A0=C2=A0=C2=A0 set pre_builds [list \ > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [list \ > > > > > > -=C2=A0=C2=A0=C2=A0 "$testname" \ > > > > > > -=C2=A0=C2=A0=C2=A0 "" \ > > > > > > -=C2=A0=C2=A0=C2=A0 "" \ > > > > > > -=C2=A0=C2=A0=C2=A0 {relax.s} \ > > > > > > -=C2=A0=C2=A0=C2=A0 {} \ > > > > > > -=C2=A0=C2=A0=C2=A0 "relax" \ > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ] \ > > > > > > -=C2=A0=C2=A0=C2=A0 ] > > > > > > - > > > > > > -=C2=A0=C2=A0=C2=A0 run_cc_link_tests $pre_builds > > > > > > - > > > > > > -=C2=A0=C2=A0=C2=A0 if [file exist "tmpdir/relax"] { > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 set objdump_output [run_host_cm= d "objdump" "-d tmpdir/relax"] > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if { [ regexp ".*pcaddi.*pcaddi= .*" $objdump_output] } { > > > > > > -=C2=A0=C2=A0=C2=A0 pass "loongarch relax" > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } { > > > > > > -=C2=A0=C2=A0=C2=A0 fail "loongarch relax" > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } > > > > > > -=C2=A0=C2=A0=C2=A0 } > > > > > > -=C2=A0 } > > > > > > - > > > > > > =C2=A0=C2=A0=C2=A0 run_ld_link_tests \ > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [list \ > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [list \ > > > > > > @@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] { > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 ] \ > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 "relax-align" \ > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ] \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [list \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "relax"= \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "-e 0 -= Ttext 0x10000 -Tdata 0x20000" "" \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "" \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 {relax.= s} \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [list \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [list o= bjdump -d relax.dd] \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ] \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "relax"= \ > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ] \ > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ] > > > > > > =C2=A0 =C2=A0=C2=A0=C2=A0 set objdump_flags "-s -j .data" >=20 --=20 Xi Ruoyao School of Aerospace Science and Technology, Xidian University