[PATCH v3] LoongArch: Batch-delete bytes at the end of each relax trip

[mengqinggang]

在 2025/6/17 下午2:20, WANG Xuerui 写道:
> On 6/17/25 14:14, mengqinggang wrote:
>>
>>
>> 在 2025/6/17 上午9:59, WANG Xuerui 写道:
>>> On 6/16/25 18:12, mengqinggang wrote:
>>>>
>>>> 在 2025/6/16 下午4:23, WANG Xuerui 写道:
>>>>> On 6/16/25 14:52, mengqinggang wrote:
>>>>>>
>>>>>> 在 2025/6/14 下午10:15, WANG Xuerui 写道:
>>>>>>> [snip]
>>>>>>>
>>>>>>> +static void
>>>>>>> +loongarch_relax_perform_deletes (bfd *abfd, asection *sec,
>>>>>>> +                 struct bfd_link_info *link_info)
>>>>>>>   {
>>>>>>>     unsigned int i, symcount;
>>>>>>>     bfd_vma toaddr = sec->size;
>>>>>>> @@ -4737,30 +4877,72 @@ loongarch_relax_delete_bytes (bfd *abfd,
>>>>>>>     Elf_Internal_Shdr *symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
>>>>>>>     unsigned int sec_shndx = _bfd_elf_section_from_bfd_section 
>>>>>>> (abfd, sec);
>>>>>>>     struct bfd_elf_section_data *data = elf_section_data (sec);
>>>>>>> -  bfd_byte *contents = data->this_hdr.contents;
>>>>>>> +  bfd_byte *contents = data->this_hdr.contents, *contents_end = 
>>>>>>> NULL;
>>>>>>>     struct relr_entry *relr = loongarch_elf_section_data 
>>>>>>> (sec)->relr;
>>>>>>>     struct loongarch_elf_link_hash_table *htab =
>>>>>>>       loongarch_elf_hash_table (link_info);
>>>>>>>     struct relr_entry *relr_end = NULL;
>>>>>>> +  splay_tree pdops = htab->pending_delete_ops;
>>>>>>> +  splay_tree_node node1 = NULL, node2 = NULL;
>>>>>>>       if (htab->relr_count)
>>>>>>>       relr_end = htab->relr + htab->relr_count;
>>>>>>>   -  /* Actually delete the bytes.  */
>>>>>>> -  sec->size -= count;
>>>>>>> -  memmove (contents + addr, contents + addr + count, toaddr - 
>>>>>>> addr - count);
>>>>>>> +  BFD_ASSERT (pdops != NULL);
>>>>>>> +  node1 = splay_tree_min (pdops);
>>>>>>> +
>>>>>>> +  if (node1 == NULL)
>>>>>>> +    /* No pending delete ops, nothing to do.  */
>>>>>>> +    return;
>>>>>>> +
>>>>>>> +  /* Actually delete the bytes.  For each delete op the pointer 
>>>>>>> arithmetics
>>>>>>> +     look like this:
>>>>>>> +
>>>>>>> +        node1->key -\                /- node2->key
>>>>>>> +            |<-- op1->size -->|        |
>>>>>>> +            v          v        v
>>>>>>> + ...---------------xxxxxxxxxxxxxxxxxxx---------xxxxxxxxxxx----...
>>>>>>> +                            ^
>>>>>>> +                node1->key + op1->size -/
>>>>>>
>>>>>> These pointers seem to not change after delete op. The delete op 
>>>>>> just copy (node1->key+op1->size, node2->key) to node1->key.
>>>>>>
>>>>> Sorry I didn't understand this, isn't there a "node1 = node2" 
>>>>> below in the for statement's expression 3 position? But indeed the 
>>>>> memmove destination is not node1->key as the illustration might 
>>>>> suggest otherwise, maybe I can clarify that in a v4. What do you 
>>>>> think?
>>>>
>>>> I mean the memmove cant change the values of pointers in the 
>>>> illustration, it just copy a memory.
>>>>
>>>> The illustrations seem to show (node1->key + op1->size) move to 
>>>> node2->key after delete op.
>>>>
>>>>
>>> What about this?
>>>
>>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>>> index aed1ef26d49..dc6251486c0 100644
>>> --- a/bfd/elfnn-loongarch.c
>>> +++ b/bfd/elfnn-loongarch.c
>>> @@ -4898,12 +4898,18 @@ loongarch_relax_perform_deletes (bfd *abfd, asection *sec,
>>>     /* Actually delete the bytes.  For each delete op the pointer arithmetics
>>>        look like this:
>>>   
>>> -           node1->key -\                           /- node2->key
>>> -                       |<-- op1->size -->|         |
>>> -                       v                 v         v
>>> -      ...---------------xxxxxxxxxxxxxxxxxxx---------xxxxxxxxxxx----...
>>> -                                                   ^
>>> -                           node1->key + op1->size -/
>>> +           node1->key -\                       /- node2->key
>>> +                       |<- op1->size ->|       |
>>> +                       v               v       v
>>> +      ...-DDDDDDD-------xxxxxxxxxxxxxxxxxSSSSSSSxxxxxxxxxx----...
>>> +         ^                                     ^
>>> +         |             node1->key + op1->size -/
>>> +         \- contents_end
>>> +
>>> +     where the "S" and "D" bytes are the memmove's source and destination
>>> +     respectively.  In case node1 is the first op, contents_end is initialized
>>> +     to the op's start; in case node2 == NULL, the chunk's end is the section's
>>> +     end.
>>>   
>>>        For memmove, we need to translate offsets to pointers by adding them to
>>>        `contents`.  */
>>>
>>> If it sounds good I'll send v4.
>>>
>> I'm a bit confused about (node1->key + op1->size) and node->key point 
>> to the same address.
>>
>> How about adding a new illustration of after the delete op?
>>
> Ah that's a mistake while I'm adjusting the whitespaces, it should 
> have been pointing to the first "S" byte. I don't think a new 
> illustration is necessary, apart from maybe mentioning contents_end 
> will move on to the end of now-written "D" bytes + 1. What do you think?
>
Agree, Thanks!


> Hard tabs are hard ¯\_(ツ)_/¯
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20250617/9d6cab4f/attachment-0001.htm>