From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21960 invoked by alias); 24 Apr 2015 01:55:56 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 21874 invoked by uid 89); 24 Apr 2015 01:55:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 24 Apr 2015 01:55:54 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 9C31EA0BA2; Fri, 24 Apr 2015 01:55:53 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-113.phx2.redhat.com [10.3.113.113]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3O1tquH011843; Thu, 23 Apr 2015 21:55:53 -0400 Message-ID: <5539A2A8.6050505@redhat.com> Date: Fri, 24 Apr 2015 01:55:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Jiong Wang CC: Steven Bosscher , "gcc-patches@gcc.gnu.org" , Kenneth Zadeck Subject: Re: [PATCH] PR 62173, re-shuffle insns for RTL loop invariant hoisting References: <54803EBE.2060607@arm.com> <5480B6D6.2020201@arm.com> <548EFE0D.1070808@arm.com> <548EFE55.6090901@arm.com> <54930811.1020003@arm.com> <20141218220908.GA20720@gate.crashing.org> <5494426A.9010209@naturalbridge.com> <54DB6587.1020207@naturalbridge.com> <54DB9CDB.5090304@arm.com> <552D4D61.9040100@redhat.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg01472.txt.bz2 On 04/21/2015 08:24 AM, Jiong Wang wrote: > > Jiong Wang writes: > >> 2015-04-14 18:24 GMT+01:00 Jeff Law : >>> On 04/14/2015 10:48 AM, Steven Bosscher wrote: >>>>> >>>>> So I think this stage2/3 binary difference is acceptable? >>>> >>>> >>>> No, they should be identical. If there's a difference, then there's a >>>> bug - which, it seems, you've already found, too. >>> >>> RIght. And so the natural question is how to fix. >>> >>> At first glance it would seem like having this new code ignore dependencies >>> rising from debug insns would work. >>> >>> Which then begs the question, what happens to the debug insn -- it's >>> certainly not going to be correct anymore if the transformation is made. >> >> Exactly. >> >> The debug_insn 2776 in my example is to record the base address of a >> local array. the new code is doing correctly here by not shuffling the >> operands of insn 2556 and 2557 as there is additional reference of >> reg:1473 from debug insn, although the code will still execute correctly >> if we do the transformation. >> >> my understanding to fix this: >> >> * delete the out-of-date mismatch debug_insn? as there is no guarantee >> to generate accurate debug info under -O2. >> >> IMO, this debug_insn may affect "DW_AT_location" field for variable >> descrption of "classes" in .debug_info section, but it's omitted in >> the final output already. >> >> <3><38a4d>: Abbrev Number: 137 (DW_TAG_variable) >> <38a4f> DW_AT_name : (indirect string, offset: 0x18db): classes >> <38a53> DW_AT_decl_file : 1 >> <38a54> DW_AT_decl_line : 548 >> <38a56> DW_AT_type : <0x38cb4> >> >> * update the debug_insn? if the following change is OK with dwarf standard >> >> from >> >> insn0: reg0 = fp + reg1 >> debug_insn: var_loc = reg0 + const_off >> insn1: reg2 = reg0 + const_off >> >> to >> >> insn0: reg0 = fp + const_off >> debug_insn: var_loc = reg0 + reg1 >> insn1: reg2 = reg0 + reg1 >> >> Thanks, >> > > And attachment is the new patch which will update debug_insn as > described in the second solution above. > > Now the stage2/3 binary differences on AArch64 gone away. Bootstrap OK. > > On AArch64, this patch give 600+ new rtl loop invariants found across > spec2k6 float. +4.5% perf improvement on 436.cactusADM because four new > invariants found in the critical function "regex_compile". > > The similar improvements may be achieved on other RISC backends like > powerpc/mips I guess. > > One thing to mention, for AArch64, one minor glitch in > aarch64_legitimize_address needs to be fixed to let this patch take > effect, I will send out that patch later as it's a seperate issue. > Powerpc/Mips don't have this glitch in LEGITIMIZE_ADDRESS hook, so > should be OK, and I verified the base address of local array in the > testcase given by Seb on pr62173 do hoisted on ppc64 now. I think > pr62173 is fixed on those 64bit arch by this patch. > > Thoughts? > > Thanks. > > 2015-04-21 Jiong Wang > > gcc/ > * loop-invariant.c (find_defs): Enable DF_DU_CHAIN build. > (vfp_const_iv): New hash table. > (expensive_addr_check_p): New boolean. > (init_inv_motion_data): Initialize new variables.> > (free_inv_motion_data): Release hash table. > (create_new_invariant): Set cheap_address to false for iv in > vfp_const_iv table. > (find_invariant_insn): Skip dependencies check for iv in vfp_const_iv > table. > (use_for_single_du): New function. > (reshuffle_insn_with_vfp): Likewise. > (find_invariants_bb): Call reshuffle_insn_with_vfp. > > gcc/testsuite/ > * gcc.dg/pr62173.c: New testcase. So ultimately isn't this just a specialized version of reassociation of pointer arithmetic? And if so, don't we have all the usual problems around introducing overflow? ISTM if this is going to go forward (ie, we somehow convince ourselves that this kind of reassociation is OK), then should it be made to apply on pointers in general rather than restricting to stack, frame, virtual-frame? I wish I'd looked more closely at the patch the first time around so that I could have raised these questions sooner. Jeff >