* [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
@ 2023-07-11 8:49 Jinyang He
2023-07-11 8:49 ` [PATCH 2/2] LoongArch: bfd: Add counter to get real relax region Jinyang He
2023-07-12 6:35 ` [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section mengqinggang
0 siblings, 2 replies; 11+ messages in thread
From: Jinyang He @ 2023-07-11 8:49 UTC (permalink / raw)
To: Chenghua Xu, Zhensong Liu, mengqinggang, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
When I tried "./ld-new sth." the relax does not work because the
elf_seg_map (info->output_bfd) was NULL in loongarch_elf_relax_section.
There is no need for the segment info, we can get the address through
the section info. Thus, remove this condition.
bfd/ChangeLog:
* elfnn-loongarch.c (loongarch_elf_relax_section): Remove
elf_seg_map condition.
ld/ChangeLog:
* testsuite/ld-loongarch-elf/relax.exp: Update trigger method.
* testsuite/ld-loongarch-elf/relax.dd: New test.
---
bfd/elfnn-loongarch.c | 1 -
ld/testsuite/ld-loongarch-elf/relax.dd | 6 +++++
ld/testsuite/ld-loongarch-elf/relax.exp | 35 +++++++------------------
3 files changed, 16 insertions(+), 26 deletions(-)
create mode 100644 ld/testsuite/ld-loongarch-elf/relax.dd
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, asection *sec,
|| sec->sec_flg0
|| (sec->flags & SEC_RELOC) == 0
|| sec->reloc_count == 0
- || elf_seg_map (info->output_bfd) == NULL
|| (info->disable_target_specific_optimizations
&& info->relax_pass == 0)
/* The exp_seg_relro_adjust is enum phase_enum (0x4),
diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd 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 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 @@
if [istarget loongarch64-*-*] {
- if [isbuild loongarch64-*-*] {
- set testname "loongarch relax build"
- set pre_builds [list \
- [list \
- "$testname" \
- "" \
- "" \
- {relax.s} \
- {} \
- "relax" \
- ] \
- ]
-
- run_cc_link_tests $pre_builds
-
- if [file exist "tmpdir/relax"] {
- set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
- if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
- pass "loongarch relax"
- } {
- fail "loongarch relax"
- }
- }
- }
-
run_ld_link_tests \
[list \
[list \
@@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] {
] \
"relax-align" \
] \
+ [list \
+ "relax" \
+ "-e 0 -Ttext 0x10000 -Tdata 0x20000" "" \
+ "" \
+ {relax.s} \
+ [list \
+ [list objdump -d relax.dd] \
+ ] \
+ "relax" \
+ ] \
]
set objdump_flags "-s -j .data"
--
2.33.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] LoongArch: bfd: Add counter to get real relax region
2023-07-11 8:49 [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section Jinyang He
@ 2023-07-11 8:49 ` Jinyang He
2023-07-12 6:35 ` [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section mengqinggang
1 sibling, 0 replies; 11+ messages in thread
From: Jinyang He @ 2023-07-11 8:49 UTC (permalink / raw)
To: Chenghua Xu, Zhensong Liu, mengqinggang, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
The relax action, loongarch_relax_pcala_addi and loongarch_relax_align,
may reduce the pc value of later instructions. Add counter to count
the number of bytes or instructions deleted. And recalculate the relax
region in loongarch_relax_pcala_addi.
bfd/ChangeLog:
* elfnn-loongarch.c (loongarch_elf_relax_section): Add vars
to count the deleted instructions and the maximum number of
instructions that may be deleted due to R_LARCH_ALIGN.
(loongarch_relax_pcala_addi): Adjust the relax decision range.
ld/ChangeLog:
* testsuite/ld-loongarch-elf/relax.exp: Add test.
* testsuite/ld-loongarch-elf/relax-edge1.s: New test.
* testsuite/ld-loongarch-elf/relax-edge2.s: New test.
* testsuite/ld-loongarch-elf/relax-edge1.dd: New test.
* testsuite/ld-loongarch-elf/relax-edge2.dd: New test.
---
bfd/elfnn-loongarch.c | 31 +++++++++++++++-----
ld/testsuite/ld-loongarch-elf/relax-edge1.dd | 6 ++++
ld/testsuite/ld-loongarch-elf/relax-edge1.s | 25 ++++++++++++++++
ld/testsuite/ld-loongarch-elf/relax-edge2.dd | 8 +++++
ld/testsuite/ld-loongarch-elf/relax-edge2.s | 15 ++++++++++
ld/testsuite/ld-loongarch-elf/relax.exp | 20 +++++++++++++
6 files changed, 98 insertions(+), 7 deletions(-)
create mode 100644 ld/testsuite/ld-loongarch-elf/relax-edge1.dd
create mode 100644 ld/testsuite/ld-loongarch-elf/relax-edge1.s
create mode 100644 ld/testsuite/ld-loongarch-elf/relax-edge2.dd
create mode 100644 ld/testsuite/ld-loongarch-elf/relax-edge2.s
diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 01f349a24..52f8ab764 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3700,16 +3700,19 @@ loongarch_relax_delete_bytes (bfd *abfd,
/* Relax pcalau12i,addi.d => pcaddi. */
static bool
loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
- Elf_Internal_Rela *rel_hi, bfd_vma symval)
+ Elf_Internal_Rela *rel_hi, bfd_vma symval,
+ uint32_t align_max_deleted, uint32_t *pcnt_deleted)
{
bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
Elf_Internal_Rela *rel_lo = rel_hi + 2;
uint32_t pca = bfd_get (32, abfd, contents + rel_hi->r_offset);
uint32_t add = bfd_get (32, abfd, contents + rel_lo->r_offset);
uint32_t rd = pca & 0x1f;
- bfd_vma pc = sec_addr (sec) + rel_hi->r_offset;
+ bfd_vma pc = sec_addr (sec) + rel_hi->r_offset - *pcnt_deleted * 4;
const uint32_t addi_d = 0x02c00000;
const uint32_t pcaddi = 0x18000000;
+ bfd_signed_vma low = (bfd_signed_vma)(int32_t)0xffe00000;
+ bfd_signed_vma high = (bfd_signed_vma)(int32_t)0x1ffffc - align_max_deleted;
/* Is pcalau12i + addi.d insns? */
if ((ELFNN_R_TYPE (rel_lo->r_info) != R_LARCH_PCALA_LO12)
@@ -3722,8 +3725,8 @@ loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
|| (((add >> 5) & 0x1f) != rd)
/* Can be relaxed to pcaddi? */
|| (symval & 0x3) /* 4 bytes align. */
- || ((bfd_signed_vma)(symval - pc) < (bfd_signed_vma)(int32_t)0xffe00000)
- || ((bfd_signed_vma)(symval - pc) > (bfd_signed_vma)(int32_t)0x1ffffc))
+ || ((bfd_signed_vma)(symval - pc) < low)
+ || ((bfd_signed_vma)(symval - pc) > high))
return false;
pca = pcaddi | rd;
@@ -3734,6 +3737,7 @@ loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
R_LARCH_PCREL20_S2);
rel_lo->r_info = ELFNN_R_INFO (ELFNN_R_SYM (rel_hi->r_info),
R_LARCH_DELETE);
+ (*pcnt_deleted) += 1;
return true;
}
@@ -3840,6 +3844,7 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
struct bfd_elf_section_data *data = elf_section_data (sec);
Elf_Internal_Rela *relocs;
*again = false;
+ uint32_t cnt_deleted = 0, align_max_deleted = 0;
if (bfd_link_relocatable (info)
|| sec->sec_flg0
@@ -3881,6 +3886,7 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
bool local_got = false;
char symtype;
struct elf_link_hash_entry *h = NULL;
+ bool relaxed = false;
if (r_symndx < symtab_hdr->sh_info)
{
@@ -3954,6 +3960,8 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
case R_LARCH_ALIGN:
if (2 == info->relax_pass)
loongarch_relax_align (abfd, sec, sym_sec, info, rel, symval);
+ else if (0 == info->relax_pass)
+ align_max_deleted += rel->r_addend;
break;
case R_LARCH_DELETE:
if (info->relax_pass == 1)
@@ -3961,23 +3969,32 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
loongarch_relax_delete_bytes (abfd, sec, rel->r_offset, 4, info);
rel->r_info = ELFNN_R_INFO (0, R_LARCH_NONE);
}
+ else if (0 == info->relax_pass)
+ cnt_deleted += 1;
break;
case R_LARCH_PCALA_HI20:
if (info->relax_pass == 0)
{
if (i + 4 > sec->reloc_count)
break;
- loongarch_relax_pcala_addi (abfd, sec, rel, symval);
+ relaxed = loongarch_relax_pcala_addi (abfd, sec, rel, symval,
+ align_max_deleted, &cnt_deleted);
+ /* Skip the next R_LARCH_{RELAX, PCALA_LO12, RELAX} relocs. */
+ if (relaxed)
+ i += 3;
}
break;
case R_LARCH_GOT_PC_HI20:
- if (local_got)
+ if (info->relax_pass == 0 && local_got)
{
if (i + 4 > sec->reloc_count)
break;
if (loongarch_relax_pcala_ld (abfd, sec, rel))
{
- loongarch_relax_pcala_addi (abfd, sec, rel, symval);
+ relaxed = loongarch_relax_pcala_addi (abfd, sec, rel, symval,
+ align_max_deleted, &cnt_deleted);
+ if (relaxed)
+ i += 3;
}
}
break;
diff --git a/ld/testsuite/ld-loongarch-elf/relax-edge1.dd b/ld/testsuite/ld-loongarch-elf/relax-edge1.dd
new file mode 100644
index 000000000..92836e679
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/relax-edge1.dd
@@ -0,0 +1,6 @@
+#...
+.*pcaddi.*
+.*pcalau12i.*
+.*addi.*
+.*pcaddi.*
+#pass
diff --git a/ld/testsuite/ld-loongarch-elf/relax-edge1.s b/ld/testsuite/ld-loongarch-elf/relax-edge1.s
new file mode 100644
index 000000000..df16c1384
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/relax-edge1.s
@@ -0,0 +1,25 @@
+ .data
+ .type obj1, @object
+obj1:
+ .word 0
+ .size obj1, .-obj1
+
+ .space 8
+
+ .type obj2, @object
+obj2:
+ .word 0
+ .size obj2, .-obj2
+
+ .text
+main:
+ la.local $a0, obj1
+ la.local $a0, obj3
+ la.local $a0, obj2
+
+ .bss
+ .space 0x4
+ .type obj3, @object
+obj3:
+ .word 0
+ .size obj3, .-obj3
diff --git a/ld/testsuite/ld-loongarch-elf/relax-edge2.dd b/ld/testsuite/ld-loongarch-elf/relax-edge2.dd
new file mode 100644
index 000000000..5610cc8ed
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/relax-edge2.dd
@@ -0,0 +1,8 @@
+#...
+.*nop.*
+.*nop.*
+.*nop.*
+.*nop.*
+.*pcalau12i.*
+.*addi.*
+#pass
diff --git a/ld/testsuite/ld-loongarch-elf/relax-edge2.s b/ld/testsuite/ld-loongarch-elf/relax-edge2.s
new file mode 100644
index 000000000..c2a42e705
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/relax-edge2.s
@@ -0,0 +1,15 @@
+ .text
+main:
+ nop
+ nop
+ nop
+ nop
+ .align 4
+ la.local $a0, obj1
+
+ .data
+ .space 0x10
+ .type obj1, @object
+obj1:
+ .word 0
+ .size obj1, .-obj1
diff --git a/ld/testsuite/ld-loongarch-elf/relax.exp b/ld/testsuite/ld-loongarch-elf/relax.exp
index d2bb967c6..19ade4970 100644
--- a/ld/testsuite/ld-loongarch-elf/relax.exp
+++ b/ld/testsuite/ld-loongarch-elf/relax.exp
@@ -43,6 +43,26 @@ if [istarget loongarch64-*-*] {
] \
"relax" \
] \
+ [list \
+ "relax-edge1" \
+ "-e 0 -Ttext 0x400000 -Tdata 0x200000 -Tbss 0x600000" "" \
+ "" \
+ {relax-edge1.s} \
+ [list \
+ [list objdump -d relax-edge1.dd] \
+ ] \
+ "relax-edge1" \
+ ] \
+ [list \
+ "relax-edge2" \
+ "-e 0 -Ttext 0x400000 -Tdata 0x600000" "" \
+ "" \
+ {relax-edge2.s} \
+ [list \
+ [list objdump -d relax-edge2.dd] \
+ ] \
+ "relax-edge2" \
+ ] \
]
set objdump_flags "-s -j .data"
--
2.33.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-07-11 8:49 [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section Jinyang He
2023-07-11 8:49 ` [PATCH 2/2] LoongArch: bfd: Add counter to get real relax region Jinyang He
@ 2023-07-12 6:35 ` mengqinggang
2023-07-12 8:08 ` Jinyang He
1 sibling, 1 reply; 11+ messages in thread
From: mengqinggang @ 2023-07-12 6:35 UTC (permalink / raw)
To: Jinyang He, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
Hi,
Without this condition, the value of 'symbol - pc' may be differences
between
loongarch_elf_relax_section and loongarch_elf_relocate_section.
And it will cause some spec2006 test errors.
在 2023/7/11 下午4:49, Jinyang He 写道:
> When I tried "./ld-new sth." the relax does not work because the
> elf_seg_map (info->output_bfd) was NULL in loongarch_elf_relax_section.
> There is no need for the segment info, we can get the address through
> the section info. Thus, remove this condition.
>
> bfd/ChangeLog:
>
> * elfnn-loongarch.c (loongarch_elf_relax_section): Remove
> elf_seg_map condition.
>
> ld/ChangeLog:
>
> * testsuite/ld-loongarch-elf/relax.exp: Update trigger method.
> * testsuite/ld-loongarch-elf/relax.dd: New test.
> ---
> bfd/elfnn-loongarch.c | 1 -
> ld/testsuite/ld-loongarch-elf/relax.dd | 6 +++++
> ld/testsuite/ld-loongarch-elf/relax.exp | 35 +++++++------------------
> 3 files changed, 16 insertions(+), 26 deletions(-)
> create mode 100644 ld/testsuite/ld-loongarch-elf/relax.dd
>
> 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, asection *sec,
> || sec->sec_flg0
> || (sec->flags & SEC_RELOC) == 0
> || sec->reloc_count == 0
> - || elf_seg_map (info->output_bfd) == NULL
> || (info->disable_target_specific_optimizations
> && info->relax_pass == 0)
> /* The exp_seg_relro_adjust is enum phase_enum (0x4),
> diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd 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 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 @@
>
> if [istarget loongarch64-*-*] {
>
> - if [isbuild loongarch64-*-*] {
> - set testname "loongarch relax build"
> - set pre_builds [list \
> - [list \
> - "$testname" \
> - "" \
> - "" \
> - {relax.s} \
> - {} \
> - "relax" \
> - ] \
> - ]
> -
> - run_cc_link_tests $pre_builds
> -
> - if [file exist "tmpdir/relax"] {
> - set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
> - if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
> - pass "loongarch relax"
> - } {
> - fail "loongarch relax"
> - }
> - }
> - }
> -
> run_ld_link_tests \
> [list \
> [list \
> @@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] {
> ] \
> "relax-align" \
> ] \
> + [list \
> + "relax" \
> + "-e 0 -Ttext 0x10000 -Tdata 0x20000" "" \
> + "" \
> + {relax.s} \
> + [list \
> + [list objdump -d relax.dd] \
> + ] \
> + "relax" \
> + ] \
> ]
>
> set objdump_flags "-s -j .data"
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-07-12 6:35 ` [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section mengqinggang
@ 2023-07-12 8:08 ` Jinyang He
2023-07-13 3:13 ` mengqinggang
0 siblings, 1 reply; 11+ messages in thread
From: Jinyang He @ 2023-07-12 8:08 UTC (permalink / raw)
To: mengqinggang, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
Hi,
I'm not fimiliar with the process of output binary. I just noted that it
calls lang_relax_sections() before calling
_bfd_elf_map_sections_to_segments() in ldelf_map_segments(). Thus I
think the condition, elf_seg_map (info->output_bfd) == NULL, always be
true when first time it calls loongarch_elf_relax_section(). Actually we
can get right symbol value when relaxation, but ...
You reminder me that the symbol value will be reduced due to relaxation,
which like the instruction pc is reduced. Then we cannot get right
symbol value if the symbol in relaxable section. And I write a test,
$ cat c.s
.section ".another.text", "x"
nop
la.local $a0, sym1
sym2:
nop
.text
sym1:
nop
nop
nop
la.local $a0, sym2
$ ./gas/as-new -mabi=lp64d -o c.o c.s
$ ./ld/ld-new c.o -o c -e 0 --section-start=.another.text=0x120000000
-Ttext 0x120200000
Disassembly of section .another.text:
0000000120000000 <sym2-0x8>:
120000000: 03400000 andi $zero, $zero, 0x0
120000004: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
0000000120000008 <sym2>:
120000008: 03400000 andi $zero, $zero, 0x0
Disassembly of section .text:
0000000120200000 <sym1>:
120200000: 03400000 andi $zero, $zero, 0x0
120200004: 03400000 andi $zero, $zero, 0x0
120200008: 03400000 andi $zero, $zero, 0x0
12020000c: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
Due to the bug repaired by Patch2 and the bug reported by this test. And
I also did not see that condition in other archs. I'm not sure that the
relaxation spec2006 error is caused by that condition or those bugs.
On 2023-07-12 14:35, mengqinggang wrote:
> Hi,
>
> Without this condition, the value of 'symbol - pc' may be differences
> between
> loongarch_elf_relax_section and loongarch_elf_relocate_section.
> And it will cause some spec2006 test errors.
>
> 在 2023/7/11 下午4:49, Jinyang He 写道:
>> When I tried "./ld-new sth." the relax does not work because the
>> elf_seg_map (info->output_bfd) was NULL in loongarch_elf_relax_section.
>> There is no need for the segment info, we can get the address through
>> the section info. Thus, remove this condition.
>>
>> bfd/ChangeLog:
>>
>> * elfnn-loongarch.c (loongarch_elf_relax_section): Remove
>> elf_seg_map condition.
>>
>> ld/ChangeLog:
>>
>> * testsuite/ld-loongarch-elf/relax.exp: Update trigger method.
>> * testsuite/ld-loongarch-elf/relax.dd: New test.
>> ---
>> bfd/elfnn-loongarch.c | 1 -
>> ld/testsuite/ld-loongarch-elf/relax.dd | 6 +++++
>> ld/testsuite/ld-loongarch-elf/relax.exp | 35 +++++++------------------
>> 3 files changed, 16 insertions(+), 26 deletions(-)
>> create mode 100644 ld/testsuite/ld-loongarch-elf/relax.dd
>>
>> 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,
>> asection *sec,
>> || sec->sec_flg0
>> || (sec->flags & SEC_RELOC) == 0
>> || sec->reloc_count == 0
>> - || elf_seg_map (info->output_bfd) == NULL
>> || (info->disable_target_specific_optimizations
>> && info->relax_pass == 0)
>> /* The exp_seg_relro_adjust is enum phase_enum (0x4),
>> diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd
>> 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
>> 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 @@
>> if [istarget loongarch64-*-*] {
>> - if [isbuild loongarch64-*-*] {
>> - set testname "loongarch relax build"
>> - set pre_builds [list \
>> - [list \
>> - "$testname" \
>> - "" \
>> - "" \
>> - {relax.s} \
>> - {} \
>> - "relax" \
>> - ] \
>> - ]
>> -
>> - run_cc_link_tests $pre_builds
>> -
>> - if [file exist "tmpdir/relax"] {
>> - set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
>> - if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
>> - pass "loongarch relax"
>> - } {
>> - fail "loongarch relax"
>> - }
>> - }
>> - }
>> -
>> run_ld_link_tests \
>> [list \
>> [list \
>> @@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] {
>> ] \
>> "relax-align" \
>> ] \
>> + [list \
>> + "relax" \
>> + "-e 0 -Ttext 0x10000 -Tdata 0x20000" "" \
>> + "" \
>> + {relax.s} \
>> + [list \
>> + [list objdump -d relax.dd] \
>> + ] \
>> + "relax" \
>> + ] \
>> ]
>> set objdump_flags "-s -j .data"
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-07-12 8:08 ` Jinyang He
@ 2023-07-13 3:13 ` mengqinggang
2023-07-13 7:19 ` Jinyang He
0 siblings, 1 reply; 11+ messages in thread
From: mengqinggang @ 2023-07-13 3:13 UTC (permalink / raw)
To: Jinyang He, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
Hi,
I will apply this two patches to see if there are any errors in specc2006.
在 2023/7/12 下午4:08, Jinyang He 写道:
> Hi,
>
>
> I'm not fimiliar with the process of output binary. I just noted that
> it calls lang_relax_sections() before calling
> _bfd_elf_map_sections_to_segments() in ldelf_map_segments(). Thus I
> think the condition, elf_seg_map (info->output_bfd) == NULL, always be
> true when first time it calls loongarch_elf_relax_section(). Actually
> we can get right symbol value when relaxation, but ...
>
> You reminder me that the symbol value will be reduced due to
> relaxation, which like the instruction pc is reduced. Then we cannot
> get right symbol value if the symbol in relaxable section. And I write
> a test,
>
> $ cat c.s
> .section ".another.text", "x"
> nop
> la.local $a0, sym1
> sym2:
> nop
>
> .text
> sym1:
> nop
> nop
> nop
> la.local $a0, sym2
>
> $ ./gas/as-new -mabi=lp64d -o c.o c.s
> $ ./ld/ld-new c.o -o c -e 0 --section-start=.another.text=0x120000000
> -Ttext 0x120200000
>
> Disassembly of section .another.text:
>
> 0000000120000000 <sym2-0x8>:
> 120000000: 03400000 andi $zero, $zero, 0x0
> 120000004: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
>
> 0000000120000008 <sym2>:
> 120000008: 03400000 andi $zero, $zero, 0x0
>
> Disassembly of section .text:
>
> 0000000120200000 <sym1>:
> 120200000: 03400000 andi $zero, $zero, 0x0
> 120200004: 03400000 andi $zero, $zero, 0x0
> 120200008: 03400000 andi $zero, $zero, 0x0
> 12020000c: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
>
> Due to the bug repaired by Patch2 and the bug reported by this test.
> And I also did not see that condition in other archs. I'm not sure
> that the relaxation spec2006 error is caused by that condition or
> those bugs.
>
>
> On 2023-07-12 14:35, mengqinggang wrote:
>> Hi,
>>
>> Without this condition, the value of 'symbol - pc' may be differences
>> between
>> loongarch_elf_relax_section and loongarch_elf_relocate_section.
>> And it will cause some spec2006 test errors.
>>
>> 在 2023/7/11 下午4:49, Jinyang He 写道:
>>> When I tried "./ld-new sth." the relax does not work because the
>>> elf_seg_map (info->output_bfd) was NULL in loongarch_elf_relax_section.
>>> There is no need for the segment info, we can get the address through
>>> the section info. Thus, remove this condition.
>>>
>>> bfd/ChangeLog:
>>>
>>> * elfnn-loongarch.c (loongarch_elf_relax_section): Remove
>>> elf_seg_map condition.
>>>
>>> ld/ChangeLog:
>>>
>>> * testsuite/ld-loongarch-elf/relax.exp: Update trigger method.
>>> * testsuite/ld-loongarch-elf/relax.dd: New test.
>>> ---
>>> bfd/elfnn-loongarch.c | 1 -
>>> ld/testsuite/ld-loongarch-elf/relax.dd | 6 +++++
>>> ld/testsuite/ld-loongarch-elf/relax.exp | 35
>>> +++++++------------------
>>> 3 files changed, 16 insertions(+), 26 deletions(-)
>>> create mode 100644 ld/testsuite/ld-loongarch-elf/relax.dd
>>>
>>> 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,
>>> asection *sec,
>>> || sec->sec_flg0
>>> || (sec->flags & SEC_RELOC) == 0
>>> || sec->reloc_count == 0
>>> - || elf_seg_map (info->output_bfd) == NULL
>>> || (info->disable_target_specific_optimizations
>>> && info->relax_pass == 0)
>>> /* The exp_seg_relro_adjust is enum phase_enum (0x4),
>>> diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd
>>> 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
>>> 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 @@
>>> if [istarget loongarch64-*-*] {
>>> - if [isbuild loongarch64-*-*] {
>>> - set testname "loongarch relax build"
>>> - set pre_builds [list \
>>> - [list \
>>> - "$testname" \
>>> - "" \
>>> - "" \
>>> - {relax.s} \
>>> - {} \
>>> - "relax" \
>>> - ] \
>>> - ]
>>> -
>>> - run_cc_link_tests $pre_builds
>>> -
>>> - if [file exist "tmpdir/relax"] {
>>> - set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
>>> - if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
>>> - pass "loongarch relax"
>>> - } {
>>> - fail "loongarch relax"
>>> - }
>>> - }
>>> - }
>>> -
>>> run_ld_link_tests \
>>> [list \
>>> [list \
>>> @@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] {
>>> ] \
>>> "relax-align" \
>>> ] \
>>> + [list \
>>> + "relax" \
>>> + "-e 0 -Ttext 0x10000 -Tdata 0x20000" "" \
>>> + "" \
>>> + {relax.s} \
>>> + [list \
>>> + [list objdump -d relax.dd] \
>>> + ] \
>>> + "relax" \
>>> + ] \
>>> ]
>>> set objdump_flags "-s -j .data"
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-07-13 3:13 ` mengqinggang
@ 2023-07-13 7:19 ` Jinyang He
2023-07-13 8:18 ` Jinyang He
0 siblings, 1 reply; 11+ messages in thread
From: Jinyang He @ 2023-07-13 7:19 UTC (permalink / raw)
To: mengqinggang, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
Hi,
We should pay attention to them, the bug in relaxation. Relaxation may
not work correctly because of the bug1 reported by the second patch and
the bug2 reported in the my previous email, although it works well in
most of the time. Those 2 bugs are just what we've found, but there may
be hidden problems.
I also tested the bug2 on RISCV and although it couldn't be resolved, it
reported an error.
Test is,
$ cat c.s
.section ".another.text", "x"
.fill 1, 4, 0
call fn2
call fn2
fn1:
nop
.text
fn2:
.fill 4, 4, 0
call fn1
$ ./gas/as-new -march=rv32ic_zicsr -mno-arch-attr -o c.o c.s
$ ./ld/ld-new c.o -o c -e 0 --section-start=.another.text=0x1000000
-Ttext 0x1100000
c.o: in function `fn2':
(.text+0x10): relocation truncated to fit: R_RISCV_JAL against `fn1'
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
important than these two patches so that we can get feedback when we link.
Thanks!
On 2023-07-13 11:13, mengqinggang wrote:
> Hi,
>
> I will apply this two patches to see if there are any errors in
> specc2006.
>
> 在 2023/7/12 下午4:08, Jinyang He 写道:
>> Hi,
>>
>>
>> I'm not fimiliar with the process of output binary. I just noted that
>> it calls lang_relax_sections() before calling
>> _bfd_elf_map_sections_to_segments() in ldelf_map_segments(). Thus I
>> think the condition, elf_seg_map (info->output_bfd) == NULL, always
>> be true when first time it calls loongarch_elf_relax_section().
>> Actually we can get right symbol value when relaxation, but ...
>>
>> You reminder me that the symbol value will be reduced due to
>> relaxation, which like the instruction pc is reduced. Then we cannot
>> get right symbol value if the symbol in relaxable section. And I
>> write a test,
>>
>> $ cat c.s
>> .section ".another.text", "x"
>> nop
>> la.local $a0, sym1
>> sym2:
>> nop
>>
>> .text
>> sym1:
>> nop
>> nop
>> nop
>> la.local $a0, sym2
>>
>> $ ./gas/as-new -mabi=lp64d -o c.o c.s
>> $ ./ld/ld-new c.o -o c -e 0 --section-start=.another.text=0x120000000
>> -Ttext 0x120200000
>>
>> Disassembly of section .another.text:
>>
>> 0000000120000000 <sym2-0x8>:
>> 120000000: 03400000 andi $zero, $zero, 0x0
>> 120000004: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
>>
>> 0000000120000008 <sym2>:
>> 120000008: 03400000 andi $zero, $zero, 0x0
>>
>> Disassembly of section .text:
>>
>> 0000000120200000 <sym1>:
>> 120200000: 03400000 andi $zero, $zero, 0x0
>> 120200004: 03400000 andi $zero, $zero, 0x0
>> 120200008: 03400000 andi $zero, $zero, 0x0
>> 12020000c: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
>>
>> Due to the bug repaired by Patch2 and the bug reported by this test.
>> And I also did not see that condition in other archs. I'm not sure
>> that the relaxation spec2006 error is caused by that condition or
>> those bugs.
>>
>>
>> On 2023-07-12 14:35, mengqinggang wrote:
>>> Hi,
>>>
>>> Without this condition, the value of 'symbol - pc' may be
>>> differences between
>>> loongarch_elf_relax_section and loongarch_elf_relocate_section.
>>> And it will cause some spec2006 test errors.
>>>
>>> 在 2023/7/11 下午4:49, Jinyang He 写道:
>>>> When I tried "./ld-new sth." the relax does not work because the
>>>> elf_seg_map (info->output_bfd) was NULL in
>>>> loongarch_elf_relax_section.
>>>> There is no need for the segment info, we can get the address through
>>>> the section info. Thus, remove this condition.
>>>>
>>>> bfd/ChangeLog:
>>>>
>>>> * elfnn-loongarch.c (loongarch_elf_relax_section): Remove
>>>> elf_seg_map condition.
>>>>
>>>> ld/ChangeLog:
>>>>
>>>> * testsuite/ld-loongarch-elf/relax.exp: Update trigger method.
>>>> * testsuite/ld-loongarch-elf/relax.dd: New test.
>>>> ---
>>>> bfd/elfnn-loongarch.c | 1 -
>>>> ld/testsuite/ld-loongarch-elf/relax.dd | 6 +++++
>>>> ld/testsuite/ld-loongarch-elf/relax.exp | 35
>>>> +++++++------------------
>>>> 3 files changed, 16 insertions(+), 26 deletions(-)
>>>> create mode 100644 ld/testsuite/ld-loongarch-elf/relax.dd
>>>>
>>>> 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,
>>>> asection *sec,
>>>> || sec->sec_flg0
>>>> || (sec->flags & SEC_RELOC) == 0
>>>> || sec->reloc_count == 0
>>>> - || elf_seg_map (info->output_bfd) == NULL
>>>> || (info->disable_target_specific_optimizations
>>>> && info->relax_pass == 0)
>>>> /* The exp_seg_relro_adjust is enum phase_enum (0x4),
>>>> diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd
>>>> 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
>>>> 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 @@
>>>> if [istarget loongarch64-*-*] {
>>>> - if [isbuild loongarch64-*-*] {
>>>> - set testname "loongarch relax build"
>>>> - set pre_builds [list \
>>>> - [list \
>>>> - "$testname" \
>>>> - "" \
>>>> - "" \
>>>> - {relax.s} \
>>>> - {} \
>>>> - "relax" \
>>>> - ] \
>>>> - ]
>>>> -
>>>> - run_cc_link_tests $pre_builds
>>>> -
>>>> - if [file exist "tmpdir/relax"] {
>>>> - set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
>>>> - if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
>>>> - pass "loongarch relax"
>>>> - } {
>>>> - fail "loongarch relax"
>>>> - }
>>>> - }
>>>> - }
>>>> -
>>>> run_ld_link_tests \
>>>> [list \
>>>> [list \
>>>> @@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] {
>>>> ] \
>>>> "relax-align" \
>>>> ] \
>>>> + [list \
>>>> + "relax" \
>>>> + "-e 0 -Ttext 0x10000 -Tdata 0x20000" "" \
>>>> + "" \
>>>> + {relax.s} \
>>>> + [list \
>>>> + [list objdump -d relax.dd] \
>>>> + ] \
>>>> + "relax" \
>>>> + ] \
>>>> ]
>>>> set objdump_flags "-s -j .data"
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-07-13 7:19 ` Jinyang He
@ 2023-07-13 8:18 ` Jinyang He
2023-10-05 10:09 ` Xi Ruoyao
0 siblings, 1 reply; 11+ messages in thread
From: Jinyang He @ 2023-07-13 8:18 UTC (permalink / raw)
To: mengqinggang, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
On 2023-07-13 15:19, Jinyang He wrote:
> Hi,
>
>
> We should pay attention to them, the bug in relaxation. Relaxation may
> not work correctly because of the bug1 reported by the second patch
> and the bug2 reported in the my previous email, although it works well
> in most of the time. Those 2 bugs are just what we've found, but there
> may be hidden problems.
> I also tested the bug2 on RISCV and although it couldn't be resolved,
> it reported an error.
>
> Test is,
> $ cat c.s
> .section ".another.text", "x"
> .fill 1, 4, 0
> call fn2
> call fn2
> fn1:
> nop
>
> .text
> fn2:
> .fill 4, 4, 0
> call fn1
>
> $ ./gas/as-new -march=rv32ic_zicsr -mno-arch-attr -o c.o c.s
> $ ./ld/ld-new c.o -o c -e 0 --section-start=.another.text=0x1000000
> -Ttext 0x1100000
> c.o: in function `fn2':
> (.text+0x10): relocation truncated to fit: R_RISCV_JAL against `fn1'
>
> 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
> important than these two patches so that we can get feedback when we
> link.
>
Now, I find that the sign-overflow-check has bug, include
reloc_bits_pcrel20_s2(), reloc_bits_b26() and (others, I did not test
them). And I think the "mask" should be
bfd_signed_vma mask = ((bfd_signed_vma)0x1 << (howto->bitsize -
(sign ? 1 : 0)) - 1;
The test about reloc_bits_b26() is,
$ cat d.s
.section ".another.text", "x"
b sym1
.text
nop
sym1:
nop
$ ./gas/as-new -mabi=lp64d -o d.o d.s
$ ./ld/ld-new d.o -o d -e 0 --section-start=.another.text=0x130000000
-Ttext 0x120000000
I'll do fix and send v2 after you test spec2006. Or other responses?
Thanks!
>
> On 2023-07-13 11:13, mengqinggang wrote:
>> Hi,
>>
>> I will apply this two patches to see if there are any errors in
>> specc2006.
>>
>> 在 2023/7/12 下午4:08, Jinyang He 写道:
>>> Hi,
>>>
>>>
>>> I'm not fimiliar with the process of output binary. I just noted
>>> that it calls lang_relax_sections() before calling
>>> _bfd_elf_map_sections_to_segments() in ldelf_map_segments(). Thus I
>>> think the condition, elf_seg_map (info->output_bfd) == NULL, always
>>> be true when first time it calls loongarch_elf_relax_section().
>>> Actually we can get right symbol value when relaxation, but ...
>>>
>>> You reminder me that the symbol value will be reduced due to
>>> relaxation, which like the instruction pc is reduced. Then we cannot
>>> get right symbol value if the symbol in relaxable section. And I
>>> write a test,
>>>
>>> $ cat c.s
>>> .section ".another.text", "x"
>>> nop
>>> la.local $a0, sym1
>>> sym2:
>>> nop
>>>
>>> .text
>>> sym1:
>>> nop
>>> nop
>>> nop
>>> la.local $a0, sym2
>>>
>>> $ ./gas/as-new -mabi=lp64d -o c.o c.s
>>> $ ./ld/ld-new c.o -o c -e 0
>>> --section-start=.another.text=0x120000000 -Ttext 0x120200000
>>>
>>> Disassembly of section .another.text:
>>>
>>> 0000000120000000 <sym2-0x8>:
>>> 120000000: 03400000 andi $zero, $zero, 0x0
>>> 120000004: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
>>>
>>> 0000000120000008 <sym2>:
>>> 120000008: 03400000 andi $zero, $zero, 0x0
>>>
>>> Disassembly of section .text:
>>>
>>> 0000000120200000 <sym1>:
>>> 120200000: 03400000 andi $zero, $zero, 0x0
>>> 120200004: 03400000 andi $zero, $zero, 0x0
>>> 120200008: 03400000 andi $zero, $zero, 0x0
>>> 12020000c: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
>>>
>>> Due to the bug repaired by Patch2 and the bug reported by this test.
>>> And I also did not see that condition in other archs. I'm not sure
>>> that the relaxation spec2006 error is caused by that condition or
>>> those bugs.
>>>
>>>
>>> On 2023-07-12 14:35, mengqinggang wrote:
>>>> Hi,
>>>>
>>>> Without this condition, the value of 'symbol - pc' may be
>>>> differences between
>>>> loongarch_elf_relax_section and loongarch_elf_relocate_section.
>>>> And it will cause some spec2006 test errors.
>>>>
>>>> 在 2023/7/11 下午4:49, Jinyang He 写道:
>>>>> When I tried "./ld-new sth." the relax does not work because the
>>>>> elf_seg_map (info->output_bfd) was NULL in
>>>>> loongarch_elf_relax_section.
>>>>> There is no need for the segment info, we can get the address through
>>>>> the section info. Thus, remove this condition.
>>>>>
>>>>> bfd/ChangeLog:
>>>>>
>>>>> * elfnn-loongarch.c (loongarch_elf_relax_section): Remove
>>>>> elf_seg_map condition.
>>>>>
>>>>> ld/ChangeLog:
>>>>>
>>>>> * testsuite/ld-loongarch-elf/relax.exp: Update trigger method.
>>>>> * testsuite/ld-loongarch-elf/relax.dd: New test.
>>>>> ---
>>>>> bfd/elfnn-loongarch.c | 1 -
>>>>> ld/testsuite/ld-loongarch-elf/relax.dd | 6 +++++
>>>>> ld/testsuite/ld-loongarch-elf/relax.exp | 35
>>>>> +++++++------------------
>>>>> 3 files changed, 16 insertions(+), 26 deletions(-)
>>>>> create mode 100644 ld/testsuite/ld-loongarch-elf/relax.dd
>>>>>
>>>>> 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,
>>>>> asection *sec,
>>>>> || sec->sec_flg0
>>>>> || (sec->flags & SEC_RELOC) == 0
>>>>> || sec->reloc_count == 0
>>>>> - || elf_seg_map (info->output_bfd) == NULL
>>>>> || (info->disable_target_specific_optimizations
>>>>> && info->relax_pass == 0)
>>>>> /* The exp_seg_relro_adjust is enum phase_enum (0x4),
>>>>> diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd
>>>>> 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
>>>>> 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 @@
>>>>> if [istarget loongarch64-*-*] {
>>>>> - if [isbuild loongarch64-*-*] {
>>>>> - set testname "loongarch relax build"
>>>>> - set pre_builds [list \
>>>>> - [list \
>>>>> - "$testname" \
>>>>> - "" \
>>>>> - "" \
>>>>> - {relax.s} \
>>>>> - {} \
>>>>> - "relax" \
>>>>> - ] \
>>>>> - ]
>>>>> -
>>>>> - run_cc_link_tests $pre_builds
>>>>> -
>>>>> - if [file exist "tmpdir/relax"] {
>>>>> - set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
>>>>> - if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
>>>>> - pass "loongarch relax"
>>>>> - } {
>>>>> - fail "loongarch relax"
>>>>> - }
>>>>> - }
>>>>> - }
>>>>> -
>>>>> run_ld_link_tests \
>>>>> [list \
>>>>> [list \
>>>>> @@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] {
>>>>> ] \
>>>>> "relax-align" \
>>>>> ] \
>>>>> + [list \
>>>>> + "relax" \
>>>>> + "-e 0 -Ttext 0x10000 -Tdata 0x20000" "" \
>>>>> + "" \
>>>>> + {relax.s} \
>>>>> + [list \
>>>>> + [list objdump -d relax.dd] \
>>>>> + ] \
>>>>> + "relax" \
>>>>> + ] \
>>>>> ]
>>>>> set objdump_flags "-s -j .data"
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-07-13 8:18 ` Jinyang He
@ 2023-10-05 10:09 ` Xi Ruoyao
2023-10-05 11:19 ` Xi Ruoyao
0 siblings, 1 reply; 11+ messages in thread
From: Xi Ruoyao @ 2023-10-05 10:09 UTC (permalink / raw)
To: Jinyang He, mengqinggang, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
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:
>
> > Hi,
> >
> >
> > We should pay attention to them, the bug in relaxation. Relaxation may
> > not work correctly because of the bug1 reported by the second patch
> > and the bug2 reported in the my previous email, although it works well
> > in most of the time. Those 2 bugs are just what we've found, but there
> > may be hidden problems.
> > I also tested the bug2 on RISCV and although it couldn't be resolved,
> > it reported an error.
> >
> > Test is,
> > $ cat c.s
> > .section ".another.text", "x"
> > .fill 1, 4, 0
> > call fn2
> > call fn2
> > fn1:
> > nop
> >
> > .text
> > fn2:
> > .fill 4, 4, 0
> > call fn1
> >
> > $ ./gas/as-new -march=rv32ic_zicsr -mno-arch-attr -o c.o c.s
> > $ ./ld/ld-new c.o -o c -e 0 --section-start=.another.text=0x1000000
> > -Ttext 0x1100000
> > c.o: in function `fn2':
> > (.text+0x10): relocation truncated to fit: R_RISCV_JAL against `fn1'
> >
> > 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
> > important than these two patches so that we can get feedback when we
> > link.
> >
>
> Now, I find that the sign-overflow-check has bug, include
> reloc_bits_pcrel20_s2(), reloc_bits_b26() and (others, I did not test
> them). And I think the "mask" should be
> bfd_signed_vma mask = ((bfd_signed_vma)0x1 << (howto->bitsize -
> (sign ? 1 : 0)) - 1;
>
> The test about reloc_bits_b26() is,
>
> $ cat d.s
> .section ".another.text", "x"
> b sym1
>
> .text
> nop
> sym1:
> nop
>
> $ ./gas/as-new -mabi=lp64d -o d.o d.s
> $ ./ld/ld-new d.o -o d -e 0 --section-start=.another.text=0x130000000
> -Ttext 0x120000000
>
> I'll do fix and send v2 after you test spec2006. Or other responses?
>
>
> Thanks!
>
>
> >
> > On 2023-07-13 11:13, mengqinggang wrote:
> > > Hi,
> > >
> > > I will apply this two patches to see if there are any errors in
> > > specc2006.
> > >
> > > 在 2023/7/12 下午4:08, Jinyang He 写道:
> > > > Hi,
> > > >
> > > >
> > > > I'm not fimiliar with the process of output binary. I just noted
> > > > that it calls lang_relax_sections() before calling
> > > > _bfd_elf_map_sections_to_segments() in ldelf_map_segments(). Thus I
> > > > think the condition, elf_seg_map (info->output_bfd) == NULL, always
> > > > be true when first time it calls loongarch_elf_relax_section().
> > > > Actually we can get right symbol value when relaxation, but ...
> > > >
> > > > You reminder me that the symbol value will be reduced due to
> > > > relaxation, which like the instruction pc is reduced. Then we cannot
> > > > get right symbol value if the symbol in relaxable section. And I
> > > > write a test,
> > > >
> > > > $ cat c.s
> > > > .section ".another.text", "x"
> > > > nop
> > > > la.local $a0, sym1
> > > > sym2:
> > > > nop
> > > >
> > > > .text
> > > > sym1:
> > > > nop
> > > > nop
> > > > nop
> > > > la.local $a0, sym2
> > > >
> > > > $ ./gas/as-new -mabi=lp64d -o c.o c.s
> > > > $ ./ld/ld-new c.o -o c -e 0
> > > > --section-start=.another.text=0x120000000 -Ttext 0x120200000
> > > >
> > > > Disassembly of section .another.text:
> > > >
> > > > 0000000120000000 <sym2-0x8>:
> > > > 120000000: 03400000 andi $zero, $zero, 0x0
> > > > 120000004: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
> > > >
> > > > 0000000120000008 <sym2>:
> > > > 120000008: 03400000 andi $zero, $zero, 0x0
> > > >
> > > > Disassembly of section .text:
> > > >
> > > > 0000000120200000 <sym1>:
> > > > 120200000: 03400000 andi $zero, $zero, 0x0
> > > > 120200004: 03400000 andi $zero, $zero, 0x0
> > > > 120200008: 03400000 andi $zero, $zero, 0x0
> > > > 12020000c: 18ffffe4 pcaddi $a0, 524287(0x7ffff)
> > > >
> > > > Due to the bug repaired by Patch2 and the bug reported by this test.
> > > > And I also did not see that condition in other archs. I'm not sure
> > > > that the relaxation spec2006 error is caused by that condition or
> > > > those bugs.
> > > >
> > > >
> > > > On 2023-07-12 14:35, mengqinggang wrote:
> > > > > Hi,
> > > > >
> > > > > Without this condition, the value of 'symbol - pc' may be
> > > > > differences between
> > > > > loongarch_elf_relax_section and loongarch_elf_relocate_section.
> > > > > And it will cause some spec2006 test errors.
> > > > >
> > > > > 在 2023/7/11 下午4:49, Jinyang He 写道:
> > > > > > When I tried "./ld-new sth." the relax does not work because the
> > > > > > elf_seg_map (info->output_bfd) was NULL in
> > > > > > loongarch_elf_relax_section.
> > > > > > There is no need for the segment info, we can get the address through
> > > > > > the section info. Thus, remove this condition.
> > > > > >
> > > > > > bfd/ChangeLog:
> > > > > >
> > > > > > * elfnn-loongarch.c (loongarch_elf_relax_section): Remove
> > > > > > elf_seg_map condition.
> > > > > >
> > > > > > ld/ChangeLog:
> > > > > >
> > > > > > * testsuite/ld-loongarch-elf/relax.exp: Update trigger method.
> > > > > > * testsuite/ld-loongarch-elf/relax.dd: New test.
> > > > > > ---
> > > > > > bfd/elfnn-loongarch.c | 1 -
> > > > > > ld/testsuite/ld-loongarch-elf/relax.dd | 6 +++++
> > > > > > ld/testsuite/ld-loongarch-elf/relax.exp | 35
> > > > > > +++++++------------------
> > > > > > 3 files changed, 16 insertions(+), 26 deletions(-)
> > > > > > create mode 100644 ld/testsuite/ld-loongarch-elf/relax.dd
> > > > > >
> > > > > > 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,
> > > > > > asection *sec,
> > > > > > || sec->sec_flg0
> > > > > > || (sec->flags & SEC_RELOC) == 0
> > > > > > || sec->reloc_count == 0
> > > > > > - || elf_seg_map (info->output_bfd) == NULL
> > > > > > || (info->disable_target_specific_optimizations
> > > > > > && info->relax_pass == 0)
> > > > > > /* The exp_seg_relro_adjust is enum phase_enum (0x4),
> > > > > > diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd
> > > > > > 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
> > > > > > 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 @@
> > > > > > if [istarget loongarch64-*-*] {
> > > > > > - if [isbuild loongarch64-*-*] {
> > > > > > - set testname "loongarch relax build"
> > > > > > - set pre_builds [list \
> > > > > > - [list \
> > > > > > - "$testname" \
> > > > > > - "" \
> > > > > > - "" \
> > > > > > - {relax.s} \
> > > > > > - {} \
> > > > > > - "relax" \
> > > > > > - ] \
> > > > > > - ]
> > > > > > -
> > > > > > - run_cc_link_tests $pre_builds
> > > > > > -
> > > > > > - if [file exist "tmpdir/relax"] {
> > > > > > - set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
> > > > > > - if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
> > > > > > - pass "loongarch relax"
> > > > > > - } {
> > > > > > - fail "loongarch relax"
> > > > > > - }
> > > > > > - }
> > > > > > - }
> > > > > > -
> > > > > > run_ld_link_tests \
> > > > > > [list \
> > > > > > [list \
> > > > > > @@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] {
> > > > > > ] \
> > > > > > "relax-align" \
> > > > > > ] \
> > > > > > + [list \
> > > > > > + "relax" \
> > > > > > + "-e 0 -Ttext 0x10000 -Tdata 0x20000" "" \
> > > > > > + "" \
> > > > > > + {relax.s} \
> > > > > > + [list \
> > > > > > + [list objdump -d relax.dd] \
> > > > > > + ] \
> > > > > > + "relax" \
> > > > > > + ] \
> > > > > > ]
> > > > > > set objdump_flags "-s -j .data"
>
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-10-05 10:09 ` Xi Ruoyao
@ 2023-10-05 11:19 ` Xi Ruoyao
2023-10-07 1:23 ` mengqinggang
0 siblings, 1 reply; 11+ messages in thread
From: Xi Ruoyao @ 2023-10-05 11:19 UTC (permalink / raw)
To: Jinyang He, mengqinggang, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
On Thu, 2023-10-05 at 18:09 +0800, Xi Ruoyao via Binutils wrote:
> 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.
Phew. It's not only a performance issue. It's actually a *correctness*
issue because the condition also causes R_LARCH_ALIGN skipped, so some
programs depending on code alignment (for e.g. duff-device-like code
using pcaddi for eg) will be broken.
Generally skipping the entire relaxation pass for any section containing
R_LARCH_ALIGN is wrong (as we've discussed in
https://github.com/llvm/llvm-project/pull/67424). Even if we don't
really relax a thing we still *must* process R_LARCH_ALIGN.
> Sorry for disturbing you during the national holiday, feel free to defer
> the reply until the holiday ends!
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-10-05 11:19 ` Xi Ruoyao
@ 2023-10-07 1:23 ` mengqinggang
2023-10-10 13:13 ` Xi Ruoyao
0 siblings, 1 reply; 11+ messages in thread
From: mengqinggang @ 2023-10-07 1:23 UTC (permalink / raw)
To: Xi Ruoyao, Jinyang He, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
Hi,
I will send a patch in the next few days to solve this issue and other
issues
related to relaxation.
在 2023/10/5 下午7:19, Xi Ruoyao 写道:
> On Thu, 2023-10-05 at 18:09 +0800, Xi Ruoyao via Binutils wrote:
>> 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.
> Phew. It's not only a performance issue. It's actually a *correctness*
> issue because the condition also causes R_LARCH_ALIGN skipped, so some
> programs depending on code alignment (for e.g. duff-device-like code
> using pcaddi for eg) will be broken.
>
> Generally skipping the entire relaxation pass for any section containing
> R_LARCH_ALIGN is wrong (as we've discussed in
> https://github.com/llvm/llvm-project/pull/67424). Even if we don't
> really relax a thing we still *must* process R_LARCH_ALIGN.
>
>> Sorry for disturbing you during the national holiday, feel free to defer
>> the reply until the holiday ends!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
2023-10-07 1:23 ` mengqinggang
@ 2023-10-10 13:13 ` Xi Ruoyao
0 siblings, 0 replies; 11+ messages in thread
From: Xi Ruoyao @ 2023-10-10 13:13 UTC (permalink / raw)
To: mengqinggang, Jinyang He, Chenghua Xu, Zhensong Liu, WANG Xuerui
Cc: binutils, Xing Li, yala, Peng Fan
On Sat, 2023-10-07 at 09:23 +0800, mengqinggang wrote:
>
> Hi,
>
> I will send a patch in the next few days to solve this issue and other
> issues
> related to relaxation.
You can refer to a related bug report,
https://sourceware.org/bugzilla/show_bug.cgi?id=30944, in the commit
message.
Thanks!
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-10-10 13:13 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-11 8:49 [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section Jinyang He
2023-07-11 8:49 ` [PATCH 2/2] LoongArch: bfd: Add counter to get real relax region Jinyang He
2023-07-12 6:35 ` [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section mengqinggang
2023-07-12 8:08 ` Jinyang He
2023-07-13 3:13 ` mengqinggang
2023-07-13 7:19 ` Jinyang He
2023-07-13 8:18 ` Jinyang He
2023-10-05 10:09 ` Xi Ruoyao
2023-10-05 11:19 ` Xi Ruoyao
2023-10-07 1:23 ` mengqinggang
2023-10-10 13:13 ` Xi Ruoyao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).