From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x229.google.com (mail-oi1-x229.google.com [IPv6:2607:f8b0:4864:20::229]) by sourceware.org (Postfix) with ESMTPS id 51031385B524 for ; Fri, 28 Jul 2023 01:02:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 51031385B524 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-x229.google.com with SMTP id 5614622812f47-3a44fae863fso1303726b6e.0 for ; Thu, 27 Jul 2023 18:02:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20221208.gappssmtp.com; s=20221208; t=1690506150; x=1691110950; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Uh4Ehoi+h1ISgrLl7bXvwYsGvo0YVszwJILSerTUaRY=; b=AhMVJPuvI3TJy2dbSzU7xf7bxqKgRuFBGQudyKV0FmkIbV6OSPnlrfWKohKDbm+C6S 6o2ACGbay72ReEbr9jD/oknyjbHHhZPDataJ/kIE8hf0buE5D7B8kfxE1anbdiMHNEVV EKAOYR+PGyeQ0GBlkFuiWIPAXK0HxyWFDFZyysEyWkoZGGpP4yHmBOlEXG0QfJR3ZqZt Vb9/Y/crzhmr1HhFzQvam0/vqfMpIvCbYymD1O37C0aOk6A7WK5SkKciUmXukYtCC6Xy 3P8QpO01b/GUOnk8AhUqoj73cOtqdRVRwI8q0G+pF/WT20PpVly2zplLCWJ1DrY+QY1A Mwgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690506150; x=1691110950; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Uh4Ehoi+h1ISgrLl7bXvwYsGvo0YVszwJILSerTUaRY=; b=jgjCwc3Vn96YTflKlKCpRcVAHHS/hO0KfE8zBjD0zSDfEnmhGBcR/uJs70X9yyF3MR H/FM0k1/CsgIs23fTnCuD9SP4s9MoBDv3VpBPiy6XCxaWdbjIK+m14914Lj6o18YckuR u4pM4yTs9oqs5pr27hYCsfZEX0zJQGLdPrqzzi4vpkvDaV+X5ER2u8GbIrRgLpZZF3cd JWLRpDwZ7arggWZ8/at2bru9tOpxfzLbAIJBbcaLMdqd/Ijvfg6A3QTIn165BJh7eKz3 yTcrKHPEQPueVGGfpx0eRG6GRvq8C/+pzjSHqgN+C4fBJntfpC4PTS5HeCBavdfeVhVA QjNQ== X-Gm-Message-State: ABy/qLaKTrll7JtpyGDFg9xpDWezmcZ40SdGKexaUveeVh4fPsGIIB2D hp1Jy4VVg/hEGaKxuvsFvgjkzmmcQcJHMgSFtkYs1U6B+tZPYLBgY6Lw0Q== X-Google-Smtp-Source: APBJJlGzL2tUcEJGtKe4u510EFP18gl8AU5Ogr18y6kJwIuAPXLTOR9xoxqtbddVUPXRoj5vyzAIFYMRYZP0V1FjxbA= X-Received: by 2002:a05:6808:218c:b0:3a3:7b77:bb2d with SMTP id be12-20020a056808218c00b003a37b77bb2dmr1293671oib.13.1690506150008; Thu, 27 Jul 2023 18:02:30 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Nelson Chu Date: Fri, 28 Jul 2023 09:02:18 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Do not gp relax against an ABS symbol if it is far away. To: Palmer Dabbelt Cc: Joseph.Faulls@imgtec.com, binutils@sourceware.org Content-Type: multipart/alternative; boundary="0000000000008ffb46060181a21e" X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: --0000000000008ffb46060181a21e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Relaxation should shrink the text, but since the default linker script has huge data segment alignment, data after the segment alignment shouldn't be affected and shrinked, so ideally it should be safe by using default linker script. Do you have a reduced test that shows we still have a chance to get the relax error by using the default linker script? On Fri, Jul 28, 2023 at 5:08=E2=80=AFAM Palmer Dabbelt wrote: > On Thu, 27 Jul 2023 09:48:24 PDT (-0700), Joseph.Faulls@imgtec.com wrote: > > Relaxations can cause the gp to move after it has been decided to gp > > relax. Against an absolute symbol, the distance may change such that the > > offset can no longer fit in the 12-bit immediate field. > > > > bfd/ > > * elfnn-riscv.c (_bfd_riscv_relax_pc) Do not gp relax against > and ABS > > symbol if it is far away. > > ld/ > > * ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. > > * ld/testsuite/ld-riscv-elf/gp-relax-abs*: New testcases. > > --- > > bfd/elfnn-riscv.c | 12 ++++++++++++ > > ld/testsuite/ld-riscv-elf/gp-relax-abs-sym.s | 3 +++ > > ld/testsuite/ld-riscv-elf/gp-relax-abs.d | 14 ++++++++++++++ > > ld/testsuite/ld-riscv-elf/gp-relax-abs.s | 5 +++++ > > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 + > > 5 files changed, 35 insertions(+) > > create mode 100644 ld/testsuite/ld-riscv-elf/gp-relax-abs-sym.s > > create mode 100644 ld/testsuite/ld-riscv-elf/gp-relax-abs.d > > create mode 100644 ld/testsuite/ld-riscv-elf/gp-relax-abs.s > > > > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c > > index 09aa7be225e..79e29e8b272 100644 > > --- a/bfd/elfnn-riscv.c > > +++ b/bfd/elfnn-riscv.c > > @@ -4885,6 +4885,18 @@ _bfd_riscv_relax_pc (bfd *abfd ATTRIBUTE_UNUSED, > > max_alignment =3D _bfd_riscv_get_max_alignment (sec, gp); > > htab->max_alignment_for_gp =3D max_alignment; > > } > > + /* If the symbol is in the abs section, relaxation could cause > the gp > > + * to move such that the gp relocation is no longer possible. > > I'm not quite sure what you mean by GP moving -- GP's the register, so > it doesn't have a value until runtime. GP is meant to equal > __global_pointer$, but that's usually a data symbol and thus doesn't > move as a result of relaxation. > > We do have some other issues related to SHN_ABS (for example position > independent vs PC relative stuff), so it's possible there's some other > bug here? > > > + * Conservatively half the allowed distance, as it cannot be > that > > + * gp moves more than this, i.e. more than half the > instructions be > > + * deleted due to relaxation. Do this by adjusting > reserve_size. */ > > + if (sym_sec->output_section =3D=3D bfd_abs_section_ptr) > > + { > > + if (symval >=3D gp) > > + reserve_size +=3D (symval - gp) / 2; > > + else > > + reserve_size +=3D (gp - symval) / 2; > > We can have more than a factor of two from relaxation, for example from > alignment. If a hueristic is all we can do then I guess we'll have to > live with it, but I think I'd want to understand the movement > An extreme case is that all the instruction can be relaxed without any huge alignments between text and data, so I guess that's probably why the half of distance comes from. If the default linker script may cause the problem, then we will need to know more details and try to resolve it. But if this only occurs without the default linker script, then I will suggest users disable that relax patterns by the .norelax directive, or related options of compiler/assembler/linker. Or maybe we can have an option for users to reject gp relax when meeting abs symbols, if without using default linker script. However, please give a reduced case by using default linker script that will be broken in detailed, thanks a lot. Nelson > > + } > > } > > } > > > > diff --git a/ld/testsuite/ld-riscv-elf/gp-relax-abs-sym.s > b/ld/testsuite/ld-riscv-elf/gp-relax-abs-sym.s > > new file mode 100644 > > index 00000000000..a018bb3a50a > > --- /dev/null > > +++ b/ld/testsuite/ld-riscv-elf/gp-relax-abs-sym.s > > @@ -0,0 +1,3 @@ > > +.section .data > > +.globl sym > > +.set sym,0x10804 > > diff --git a/ld/testsuite/ld-riscv-elf/gp-relax-abs.d > b/ld/testsuite/ld-riscv-elf/gp-relax-abs.d > > new file mode 100644 > > index 00000000000..2c7ab3a2579 > > --- /dev/null > > +++ b/ld/testsuite/ld-riscv-elf/gp-relax-abs.d > > @@ -0,0 +1,14 @@ > > +#source: gp-relax-abs.s > > +#source: gp-relax-abs-sym.s > > +#as: -march=3Drv64ic -mabi=3Dlp64 > > +#ld: -Tcode-model-01.ld -melf64lriscv > > +#objdump: -d > > + > > +.*:[ ]+file format .* > > + > > +Disassembly of section \.text: > > + > > +[0-9a-f]+ <_start>: > > +.*auipc.* > > +.*lw.*# [0-9a-f]* > > +#pass > > diff --git a/ld/testsuite/ld-riscv-elf/gp-relax-abs.s > b/ld/testsuite/ld-riscv-elf/gp-relax-abs.s > > new file mode 100644 > > index 00000000000..db2103bafd1 > > --- /dev/null > > +++ b/ld/testsuite/ld-riscv-elf/gp-relax-abs.s > > @@ -0,0 +1,5 @@ > > +.text > > +.global _start > > +_start: > > + auipc t0, %pcrel_hi(sym) > > + lw t0, %pcrel_lo(_start)(t0) > > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > index 947a266ba72..a53a2758991 100644 > > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > @@ -172,6 +172,7 @@ if [istarget "riscv*-*-*"] { > > run_dump_test "attr-merge-priv-spec-failed-06" > > run_dump_test "attr-phdr" > > run_dump_test "relax-max-align-gp" > > + run_dump_test "gp-relax-abs" > > run_dump_test "uleb128" > > run_ld_link_tests [list \ > > [list "Weak reference 32" "-T weakref.ld > -m[riscv_choose_ilp32_emul]" "" \ > --0000000000008ffb46060181a21e--