Re: [Open64-devel] One bug in add_one_if_stmt, Can gatekeeper review this patch? Thx very much
Brought to you by:
ributzka,
suneeljain
From: ruifen S. <she...@gm...> - 2010-08-31 02:21:13
|
hi, mei. As sun said that one should never produce wrong code in any stage. I agree with your opinion. Another, If adding label_stmt in the front in an early pahse, it should be cleared or merged if the label is useless in my view. hi, sun. which block? rvi_emit? 2010/8/31 Sun Chan <sun...@gm...> > I wondered why there is such a block though. > Sun > > On Tue, Aug 31, 2010 at 3:23 AM, Ye, Mei <Me...@am...> wrote: > > Hello Ruifen, > > > > > > > > It looks like RVI_EMIT assumes an empty block should not be a branch > > target. My opinion is before “Emit_bb” is called, we should have a pass > to > > delete empty blocks and delete/insert label stmts. > > > > If you refer to “be/opt/opt_htable_emit.cxx”, search for “Fix 592011”, > > “CFG::Delete_empty_BB” is called to do similar pre-processing. > > > > Adding a label stmt up in the front in an early phase can introduce > useless > > labels. Though it probably doesn’t hurt if later phases can clean it up. > > > > > > > > -Mei > > > > > > > > ________________________________ > > > > From: ruifen Shen [mailto:she...@gm...] > > Sent: Sunday, August 29, 2010 6:31 PM > > To: Sun Chan; Ye, Mei > > Cc: open64-devel > > > > Subject: Re: [Open64-devel] One bug in add_one_if_stmt, Can gatekeeper > > review this patch? Thx very much > > > > > > > > hi, mei & sun. > > > > Thanks very much for your review. I am sorry for the later reply as I am > on > > weekend. > > > > Attached Please find the souce file(a.c) and the trace file with flags > (slcc > > a.c -Wb,-trLOW,-tt25:0xffffffff,-tt26:0xffffffff). > > > > we can see that the lab2050 is mapped with BB5 at line 12104 of a.t. But > > after the RVI_EMIT (tansform back to whirl tree), the mapping is missing. > > > > Add label_stmt at RVI_EMIT? I do not think it is a good method. > > > > Please help to analysis that. Thanks very much. > > > > 2010/8/28 Sun Chan <sun...@gm...> > > > > Mei, > > I think you are right. The assertion about RID is a hint of something > > else wrong (RID stands for region id). > > Sun > > > > On Sat, Aug 28, 2010 at 12:36 AM, Ye, Mei <Me...@am...> wrote: > >> My guess is that by adding a label statement you may have prevented the > >> elimination of an empty block or something similar which then preserves > >> the > >> label-BB map and thus avoids the assertion. > >> > >> > >> > >> -Mei > >> > >> > >> > >> ________________________________ > >> > >> From: Ye, Mei [mailto:Me...@am...] > >> Sent: Friday, August 27, 2010 9:25 AM > >> To: ruifen Shen; open64-devel > >> Subject: Re: [Open64-devel] One bug in add_one_if_stmt, Can gatekeeper > >> review this patch? Thx very much > >> > >> > >> > >> Hello Ruifen > >> > >> > >> > >> I am not convinced that a label statement is needed here. > >> > >> It is likely that somewhere downstream after some sort of > transformations > >> a > >> different bug misses a mapping between a label and a BB which then > >> triggers > >> the assertion. > >> > >> > >> > >> -Mei > >> > >> > >> > >> ________________________________ > >> > >> From: ruifen Shen [mailto:she...@gm...] > >> Sent: Friday, August 27, 2010 2:36 AM > >> To: open64-devel > >> Subject: [Open64-devel] One bug in add_one_if_stmt, Can gatekeeper > review > >> this patch? Thx very much > >> > >> > >> > >> hi, all. > >> > >> > >> > >> We found One bug in add_one_if_stmt (opt_cfg.cxx). Can gatekeeper give a > >> reivew? Thanks very much. > >> > >> > >> > >> When create the else_bb, it just create label_map without label_stmt. > >> > >> Then in the later phase. if there is check_label for BB(such as dce, > >> reconstruction-cfg), the label_stmt will be added. > >> > >> Othere wise, the label will be missed at cg phase. Assertion will be > >> reported like followed. > >> > >> > >> > >> ### Assertion failure at line 5392 of > >> /home/test/regression/open64-SL/osprey/be/cg/whirl2ops.cxx: > >> ### Compiler Error in file a.c during Code_Expansion phase: > >> ### RID == NULL, label 2050 doesn't have a matching target > >> > >> > >> > >> I would like to fix the bug as followed. > >> > >> > >> > >> Index: opt_cfg.cxx > >> =================================================================== > >> --- opt_cfg.cxx (revision 3323) > >> +++ opt_cfg.cxx (working copy) > >> @@ -2298,8 +2298,7 @@ > >> CFG::Add_one_if_stmt( WN *wn, END_BLOCK *ends_bb ) > >> { > >> // create, but do not connect, the "else" block > >> - BB_NODE *else_bb = Create_bb(); > >> - Append_label_map(Alloc_label(), else_bb); > >> + BB_NODE *else_bb = Create_labelled_bb(); > >> > >> // create if bb > >> WN *end_cond = WN_CreateFalsebr(else_bb->Labnam(), WN_if_test(wn)); > >> > >> > >> > >> -- > >> Best Regards. > >> > >> Shen Ruifen > >> > >> tel: 010-51266989-226 > >> > > > >> > >> > ------------------------------------------------------------------------------ > >> Sell apps to millions through the Intel(R) Atom(Tm) Developer Program > >> Be part of this innovative community and reach millions of netbook users > >> worldwide. Take advantage of special opportunities to increase revenue > and > >> speed time-to-market. Join now, and jumpstart your future. > >> http://p.sf.net/sfu/intel-atom-d2d > >> _______________________________________________ > >> Open64-devel mailing list > >> Ope...@li... > >> https://lists.sourceforge.net/lists/listinfo/open64-devel > >> > >> > > > > > > -- > > Best Regards. > > > > Shen Ruifen > > > > tel: 010-51266989-226 > -- Best Regards. Shen Ruifen tel: 010-51266989-226 |