From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89093 invoked by alias); 4 Aug 2016 05:50:12 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 89083 invoked by uid 89); 4 Aug 2016 05:50:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=discount, H*Ad:U*davidxl, integral, Sri X-HELO: mail-qk0-f196.google.com Received: from mail-qk0-f196.google.com (HELO mail-qk0-f196.google.com) (209.85.220.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 04 Aug 2016 05:50:01 +0000 Received: by mail-qk0-f196.google.com with SMTP id q62so18712314qkf.2 for ; Wed, 03 Aug 2016 22:50:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9KjzeUVKe3CJIdFWKfqLe489rJi8csu5nQfC1WxdQ2c=; b=mHAaMpC4FXqH3xYj/q199hkoxzTOLZO8Sc8Ve6nsDBLzxiXlqKu1RwtNDxjPK8EeoC 4ACm9kxyDB8VMt2IwWr+APR6LV4w8Z/RFR7hzCu2lImNBCnwQUxahbQpMXlV98riB5/5 JN29WPIqPjibLJnfztFP7ipLz08LxuKXEmxSMxaouh5amcGwFloMo3tQj6W8KQazCNcz xHR/nZbIb8LoAEiY67Ka8xTeG67Bqfp/ga/2VkvflwkPT/ckZtb8CdjoAzpW0UDdLj4H T/hl/49vCNZbqm4EBpWh5sJEFaZTkpGUE1WdgO9OXOesAwFO1b6japPxIkoR0W3KOdDx 4nrA== X-Gm-Message-State: AEkoouup5YGRzNh3L6hr/Rd3aAa8VUySfPWuw59M0Vl6J5iHg4oTz1JAuH8wWnosS6l2Vf8EhR9YQslTse+48Q== X-Received: by 10.55.42.99 with SMTP id q96mr4379627qkh.246.1470289798803; Wed, 03 Aug 2016 22:49:58 -0700 (PDT) MIME-Version: 1.0 Received: by 10.55.20.8 with HTTP; Wed, 3 Aug 2016 22:49:58 -0700 (PDT) In-Reply-To: References: <20160718055056.GP1256@bubble.grove.modra.org> From: Cary Coutant Date: Thu, 04 Aug 2016 05:50:00 -0000 Message-ID: Subject: Re: [PATCH] PR gold/17704 To: Sriraman Tallam Cc: Alan Modra , binutils , David Li , Roland McGrath , ngg@tresorit.com Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-08/txt/msg00026.txt.bz2 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 wrote: > On Thu, Jul 21, 2016 at 11:11 AM, Sriraman Tallam wrote: >> On Sun, Jul 17, 2016 at 10:50 PM, Alan Modra 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