From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12950 invoked by alias); 21 Oct 2011 09:13:17 -0000 Received: (qmail 12938 invoked by uid 22791); 21 Oct 2011 09:13:16 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,TW_TM X-Spam-Check-By: sourceware.org Received: from mail-qw0-f47.google.com (HELO mail-qw0-f47.google.com) (209.85.216.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 21 Oct 2011 09:13:01 +0000 Received: by qam2 with SMTP id 2so3081821qam.20 for ; Fri, 21 Oct 2011 02:13:00 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.119.2 with SMTP id kq2mr930958obb.41.1319188380381; Fri, 21 Oct 2011 02:13:00 -0700 (PDT) Received: by 10.182.12.202 with HTTP; Fri, 21 Oct 2011 02:13:00 -0700 (PDT) In-Reply-To: <4EA026CB.2060309@mentor.com> References: <4EA026CB.2060309@mentor.com> Date: Fri, 21 Oct 2011 09:31:00 -0000 Message-ID: Subject: Re: [PATCH, PR50763] Fix for ICE in verify_gimple From: Richard Guenther To: Tom de Vries Cc: "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes 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 X-SW-Source: 2011-10/txt/msg01938.txt.bz2 On Thu, Oct 20, 2011 at 3:48 PM, Tom de Vries wrot= e: > Richard, > > I have a fix for PR50763. > > The second example from the PR looks like this: > ... > int bar (int i); > > void > foo (int c, int d) > { > =A0if (bar (c)) > =A0 =A0bar (c); > =A0d =3D 33; > =A0while (c =3D=3D d); > } > ... > > When compiled with -O2 -fno-dominator-opt, the gimple representation befo= re > ftree-tail-merge looks like this: > ... > foo (intD.6 cD.1606, intD.6 dD.1607) > { > =A0intD.6 D.2730; > > =A0# BLOCK 2 freq:900 > =A0# PRED: ENTRY [100.0%] =A0(fallthru,exec) > =A0# .MEMD.2733_6 =3D VDEF <.MEMD.2733_5(D)> > =A0# USE =3D nonlocal > =A0# CLB =3D nonlocal > =A0D.2730_2 =3D barD.1605 (cD.1606_1(D)); > =A0if (D.2730_2 !=3D 0) > =A0 =A0goto ; > =A0else > =A0 =A0goto ; > =A0# SUCC: 3 [29.0%] =A0(true,exec) 7 [71.0%] =A0(false,exec) > > =A0# BLOCK 7 freq:639 > =A0# PRED: 2 [71.0%] =A0(false,exec) > =A0goto ; > =A0# SUCC: 4 [100.0%] =A0(fallthru) > > =A0# BLOCK 3 freq:261 > =A0# PRED: 2 [29.0%] =A0(true,exec) > =A0# .MEMD.2733_7 =3D VDEF <.MEMD.2733_6> > =A0# USE =3D nonlocal > =A0# CLB =3D nonlocal > =A0barD.1605 (cD.1606_1(D)); > =A0# SUCC: 4 [100.0%] =A0(fallthru,exec) > > =A0# BLOCK 4 freq:900 > =A0# PRED: 7 [100.0%] =A0(fallthru) 3 [100.0%] =A0(fallthru,exec) > =A0# .MEMD.2733_4 =3D PHI <.MEMD.2733_6(7), .MEMD.2733_7(3)> > =A0if (cD.1606_1(D) =3D=3D 33) > =A0 =A0goto ; > =A0else > =A0 =A0goto ; > =A0# SUCC: 8 [91.0%] =A0(true,exec) 9 [9.0%] =A0(false,exec) > > =A0# BLOCK 9 freq:81 > =A0# PRED: 4 [9.0%] =A0(false,exec) > =A0goto ; > =A0# SUCC: 6 [100.0%] =A0(fallthru) > > =A0# BLOCK 8 freq:819 > =A0# PRED: 4 [91.0%] =A0(true,exec) > =A0# SUCC: 5 [100.0%] =A0(fallthru) > > =A0# BLOCK 5 freq:9100 > =A0# PRED: 8 [100.0%] =A0(fallthru) 10 [100.0%] =A0(fallthru) > =A0if (cD.1606_1(D) =3D=3D 33) > =A0 =A0goto ; > =A0else > =A0 =A0goto ; > =A0# SUCC: 10 [91.0%] =A0(true,exec) 11 [9.0%] =A0(false,exec) > > =A0# BLOCK 10 freq:8281 > =A0# PRED: 5 [91.0%] =A0(true,exec) > =A0goto ; > =A0# SUCC: 5 [100.0%] =A0(fallthru) > > =A0# BLOCK 11 freq:819 > =A0# PRED: 5 [9.0%] =A0(false,exec) > =A0# SUCC: 6 [100.0%] =A0(fallthru) > > =A0# BLOCK 6 freq:900 > =A0# PRED: 11 [100.0%] =A0(fallthru) 9 [100.0%] =A0(fallthru) > =A0# VUSE <.MEMD.2733_4> > =A0return; > =A0# SUCC: EXIT [100.0%] > > } > ... > > During the first iteration, tail_merge_optimize finds that block 9 and 11= , and > block 8 and 10 are equal, and removes block 11 and 10. > During the second iteration it finds that block 4 and block 5 are equal, = and it > removes block 5. > > Since pre had no effect, the responsibility for updating the vops lies wi= th > tail_merge_optimize. > > Block 4 starts with a virtual PHI which needs updating, but replace_block= _by > decides that an update is not necessary, because vop_at_entry returns NUL= L_TREE > for block 5 (the vop_at_entry for block 4 is .MEMD.2733_4). > What is different from normal is that block 4 dominates block 5. > > The patch makes sure that the vops are also updated if vop_at_entry is de= fined > for only one of bb1 and bb2. > > This also forced me to rewrite the code that updates the uses, which uses > dominator info now. This forced me to keep the dominator info up-to-date.= Which > forced me to move the actual deletion of the basic block and some additio= nal > bookkeeping related to that from purge_bbs to replace_block_by. > > Additionally, I fixed the case that update_vuses leaves virtual phis with= only > one argument (see unlink_virtual_phi). > > bootstrapped and reg-tested on x86_64. The tested patch had one addition = to the > attached patch: calling verify_dominators at the end of replace_block_by. > > OK for trunk? + if (gimple_code (stmt) !=3D GIMPLE_PHI && + !dominated_by_p (CDI_DOMINATORS, gimple_bb (stmt), bb2)) continue; &&s go to the next line please. The unlink_virtual_phi function needs a comment. Ok with that changes. Richard. > Thanks, > - Tom > > 2011-10-20 =A0Tom de Vries =A0 > > =A0 =A0 =A0 =A0PR tree-optimization/50763 > =A0 =A0 =A0 =A0* tree-ssa-tail-merge.c (same_succ_flush_bb): New function= , factored out > =A0 =A0 =A0 =A0of ... > =A0 =A0 =A0 =A0(same_succ_flush_bbs): Use same_succ_flush_bb. > =A0 =A0 =A0 =A0(purge_bbs): Remove argument. =A0Remove calls to same_succ= _flush_bbs, > =A0 =A0 =A0 =A0release_last_vdef and delete_basic_block. > =A0 =A0 =A0 =A0(unlink_virtual_phi): New function. > =A0 =A0 =A0 =A0(update_vuses): Add and use vuse1_phi_args argument. =A0Se= t var to > =A0 =A0 =A0 =A0SSA_NAME_VAR of vuse1 or vuse2, and use var. =A0Handle cas= e that def_stmt2 > =A0 =A0 =A0 =A0is NULL. =A0Use phi result as phi arg in case vuse1 or vus= e2 is NULL_TREE. > =A0 =A0 =A0 =A0Replace uses of vuse1 if vuse2 is NULL_TREE. =A0Fix code t= o limit > =A0 =A0 =A0 =A0replacement of uses. =A0Propagate phi argument for phis wi= th a single > =A0 =A0 =A0 =A0argument. > =A0 =A0 =A0 =A0(replace_block_by): Update vops if phi_vuse1 or phi_vuse2 = is NULL_TREE. > =A0 =A0 =A0 =A0Set vuse1_phi_args if vuse1 is a phi defined in bb1. =A0Ad= d vuse1_phi_args > =A0 =A0 =A0 =A0as argument to call to update_vuses. =A0Call release_last_= vdef, > =A0 =A0 =A0 =A0same_succ_flush_bb, delete_basic_block. =A0Update CDI_DOMI= NATORS info. > =A0 =A0 =A0 =A0(tail_merge_optimize): Remove argument in call to purge_bb= s. =A0Remove > =A0 =A0 =A0 =A0call to free_dominance_info. =A0Only call calculate_domina= nce_info once. > > =A0 =A0 =A0 =A0* gcc.dg/pr50763.c: New test. >