This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR gold/17704
- From: Cary Coutant <ccoutant at gmail dot com>
- To: Sriraman Tallam <tmsriram at google dot com>
- Cc: Alan Modra <amodra at gmail dot com>, binutils <binutils at sourceware dot org>, David Li <davidxl at google dot com>, Roland McGrath <mcgrathr at google dot com>, ngg at tresorit dot com
- Date: Wed, 3 Aug 2016 22:49:58 -0700
- Subject: Re: [PATCH] PR gold/17704
- Authentication-results: sourceware.org; auth=none
- References: <CAAs8HmwBLxMgozRZ2pQkUTxxBrweWc7d8eTv9mKpZDX7ucEahQ@mail.gmail.com> <20160718055056.GP1256@bubble.grove.modra.org> <CAAs8Hmxc-bqRv7Mdfqg9ssYC+XC96gCiyqDdA=JFsLR14strjg@mail.gmail.com> <CAAs8HmxFCx_GEquMA_+bfJJ7P8czpuhyEjtT_F8_SQVR2svfdA@mail.gmail.com>
Hi,
I'm back from vacation, and am trying to catch up now.
-cary
On Mon, Aug 1, 2016 at 9:56 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> On Thu, Jul 21, 2016 at 11:11 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>> On Sun, Jul 17, 2016 at 10:50 PM, Alan Modra <amodra@gmail.com> wrote:
>>> On Thu, Jul 14, 2016 at 11:48:39AM -0700, Sriraman Tallam wrote:
>>>> @@ -671,7 +667,31 @@ match_sections(unsigned int iteration_num,
>>>> this_secn_contents.c_str(),
>>>> this_secn_contents.length()) != 0)
>>>> continue;
>>>> - (*kept_section_id)[i] = kept_section;
>>>> +
>>>> + // Check section alignment here.
>>>> + // The section with the larger alignment requirement
>>>> + // should be kept. Do not fold if the larger is not
>>>> + // divisible by the smaller, but that case will never
>>>> + // happen if the alignments are powers of 2.
>>>> + uint64_t align_i = section_addraligns[i];
>>>> + uint64_t align_kept = section_addraligns[kept_section];
>>>> + if (align_i <= align_kept)
>>>> + {
>>>> + if (align_i != align_kept && align_i != 0
>>>> + && (align_kept % align_i) != 0)
>>>> + continue;
>>>> + (*kept_section_id)[i] = kept_section;
>>>> + }
>>>> + else
>>>> + {
>>>> + if (align_kept != 0 && (align_i % align_kept) != 0)
>>>> + continue;
>>>> + (*kept_section_id)[kept_section] = i;
>>>> + it->second = i;
>>>> + full_section_contents[kept_section].swap(
>>>> + full_section_contents[i]);
>>>> + }
>>>> +
>>>> converged = false;
>>>> break;
>>>> }
>>>
>>> The gabi says of sh_addralign "Currently, only 0 and positive integral
>>> powers of two are allowed", so I think you can discount the
>>> possibility of alignment not a power of two. It's not worth
>>> supporting here when other parts of gold do not, eg. see layout.cc
>>> calls to align_address.
>>>
>>> Please also fix indentation, only some of the above lines use tabs.
>>
>> New patch attached with the changes.
>>
>> PR gold/17704
>> * icf.cc (match_sections): Add new parameter section_addraligns.
>> Check section alignment and keep the section with the strictest
>> alignment.
>> (find_identical_sections): New local variable section_addraligns.
>> Store each section's alignment.
>> * testsuite/pr17704a_test.s: New file.
>> * testsuite/Makefile.am (pr17704a_test): New test.
>> * testsuite/Makefile.in: Regenerate.
>
>
> Hi Cary,
>
> Ping, in case you didnt take a look at this.
>
> Thanks
> Sri
>
>>
>> Thanks
>> Sri
>>
>>>
>>>> diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
>>>> index c8d3093..7b9b267 100644
>>>> --- a/gold/testsuite/Makefile.am
>>>> +++ b/gold/testsuite/Makefile.am
>>>> @@ -355,6 +355,12 @@ icf_sht_rel_addend_test: icf_sht_rel_addend_test_1.o icf_sht_rel_addend_test_2.o
>>>> icf_sht_rel_addend_test.stdout: icf_sht_rel_addend_test
>>>> $(TEST_NM) icf_sht_rel_addend_test > icf_sht_rel_addend_test.stdout
>>>>
>>>> +check_PROGRAMS += pr17704a_test
>>>> +pr17704a_test.o: pr17704a_test.s
>>>> + $(TEST_AS) --64 -o $@ $<
>>>> +pr17704a_test: pr17704a_test.o gcctestdir/ld
>>>> + gcctestdir/ld --icf=all -o $@ $<
>>>> +
>>>> check_PROGRAMS += large_symbol_alignment
>>>> large_symbol_alignment_SOURCES = large_symbol_alignment.cc
>>>> large_symbol_alignment_DEPENDENCIES = gcctestdir/ld
>>>
>>> The new test contains x86 assembly, but is enabled for all native
>>> targets using gcc. Please move to a more appropriate part of the
>>> makefile.
>>>
>>> --
>>> Alan Modra
>>> Australia Development Lab, IBM