This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold] Merging string literals with bigger alignment
- From: Alexander Ivchenko <aivchenk at gmail dot com>
- To: Cary Coutant <ccoutant at google dot com>
- Cc: Ian Lance Taylor <iant at google dot com>, binutils <binutils at sourceware dot org>, Sriraman Tallam <tmsriram at google dot com>
- Date: Tue, 2 Apr 2013 17:57:45 +0400
- Subject: Re: [gold] Merging string literals with bigger alignment
- References: <CACysShhXDPxUsqdUprwhyGcaNiS9X_ODbrW847wHHiwbUh4LhA at mail dot gmail dot com> <CAKOQZ8xROxuJMD19P7JoM084LDyDKk_anwiuoWRpS+2MLeThnA at mail dot gmail dot com> <CACysShhaCv68A39oTk5FfmsdRFjhoOjQEzEZys4U0-sZs-x+4A at mail dot gmail dot com> <CAKOQZ8ypF=yCMmrwn-8jSp_WBMdrNi6NqwJRqabG7ZUwV2yMhg at mail dot gmail dot com> <CACysShhyE2G_8fz1pLSjTdJ_f9OYdSHHB9zvydWPaJNHjFkQ8g at mail dot gmail dot com> <CAKOQZ8wd_7bfSfMsm_WmJnSdT_J4qF-JM+3Ex=tU0iDJCOWETQ at mail dot gmail dot com> <CACysShh22grCuoa7-b3QV7D_Q-RBYrtLH8ifr2yRcS93+GRk1g at mail dot gmail dot com> <CAHACq4pqmPp7JqwOJ_Ux0NN7yVZzq82t4uf9ZUKTa8eXL+Znrg at mail dot gmail dot com> <CACysShjRoZMEp=JasvdY9Jg=+ab15FEEUY-WnR-4dLb9q+C-Cg at mail dot gmail dot com> <CAHACq4rEinAxU_5Cp7SQiNeZN8tZsbp-YCwbuzR-bC5Z9GOrJw at mail dot gmail dot com> <CACysShh_8waYTA4tQWGnRER5xrwv=NgSn2NRhzOLfncPuOeOsg at mail dot gmail dot com> <CAHACq4odyq1ok_SWBFNUDreq8=b-4MMqp3DXJY03viF2-HUe8w at mail dot gmail dot com> <CACysShj2TB84vkATNQhDVYx7QURtx29Qpb5t+i_i2OP6R-zgZQ at mail dot gmail dot com> <CAHACq4q4=bd1gB9LEBJRwOnqDTYQxZ_Z=BKN++M0KSPqDdGJtg at mail dot gmail dot com>
>>> I think you'll also need to change
>>> Stringpool_template<Stringpool_char>::Stringpool_eq::operator() to
>>> check the alignment that you've stored in the Hashkey. Imagine one
>>> section that adds a string with alignment 1, and a subsequent section
>>> that adds the same string with alignment 4 -- when adding the string
>>> the second time, you'll match the first, and keep the 1-byte
>>> alignment. Ideally, you'd just update the alignment so there's only
>>> one copy of the string at the larger alignment, but that will only
>>> work if you're optimizing the string tables (-O2).
>>
>> I know that now two equal strings with different alignments will not
>> be merged (BFD also does not merge them together).
>> But to improve that we also need to change
>> Merge_section_properties::eq() so mergable string sections with
>> different alignments
>> would fall into the same Output_merge_string_data (we also need to
>> accordingly update some asserts, hash function for
>> Merge_section_properties etc.).
>> Since BFD doesn't do it, I want to make it as a separate subsequent
>> patch, if you don't mind of course..
>
> I understand that add_merge_input_section will group merge sections by
> their alignment, so that 1-byte aligned strings and 4-byte aligned
> strings will go into different merge sections. I'm OK with that, and
> am not asking you to change that.
>
> My point is that you've altered the Stringpool API to present an
> interface that allows strings with different alignment requirements to
> be entered into the same pool, but that it will ignore the larger
> alignment if the pool already contains the same string with a smaller
> alignment. I'd suggest: (a) make it do the right thing by entering the
> string again with the larger alignment, or (b) change the API to
> disallow this possibility. If you do (a), you could also have it
> coalesce the identical strings into the one with the larger alignment
> when optimizing the string table. If you do (b), you could either (b1)
> have it assert that the alignment is the same for every string added
> to the pool, or (b2) move the alignment from add_with_length to the
> Stringpool constructor.
>
> Since you know that add_merge_input_section will ensure that all
> strings in one pool will have the same alignment, I'd suggest (b2).
> The alignment is already passed to the Output_merge_string
> constructor, and can easily be passed to the initializer for
> stringpool_.
Now I see your point, thank you for clarification. If we choose (b2)
we would not be able to
create only one Output_merge_string_data for string literals with
different alignments (cause each
Output_merge_string_data has only one string pool and its alignment
would be fixed) and therefore
never would be able to merge the same string from, say, rodata.str1.4
and .rodata.str.1.8.
It seems like a little restriction to me (I don't know how much memory
we can get out of that), but if you are okey I'll do (b2).
thank you,
Alexander