Re: [Open64-devel] One bug in add_one_if_stmt, Can gatekeeper review this patch? Thx very much
Brought to you by:
ributzka,
suneeljain
Menu
▾
▴
Re: [Open64-devel] One bug in add_one_if_stmt, Can gatekeeper review this patch? Thx very much
|
From: ruifen S. <she...@gm...> - 2010-08-31 02:33:03
|
hi, sun.
Before if_conv(opt_cfg.cxx), the wn is like as followed.
IF {line: 0} {freq: 0, ln: 0, col: 0}
F8F8LDID 75 <1,7,.preg_F8> T<11,.predef_F8,4> # <preg>
F8F8LDID 76 <1,7,.preg_F8> T<11,.predef_F8,4> # lpre_cst
I4F8LE
THEN
BLOCK {line: 0} {freq: 0, ln: 0, col: 0}
F8F8LDID 77 <1,7,.preg_F8> T<11,.predef_F8,4> # lpre_cst
F8STID 75 <1,7,.preg_F8> T<11,.predef_F8,4> # <preg> {line: 11} {freq: 0,
ln: 11, col: 0}
END_BLOCK
ELSE
BLOCK {line: 0} {freq: 0, ln: 0, col: 0}
END_BLOCK
END_IF
after add_one_if_stmt, it is converted to
F8F8LDID 75 <st 1793> T<11,.predef_F8,4>
F8F8LDID 76 <st 1793> T<11,.predef_F8,4>
I4F8LE
FALSEBR L2050 {line: 0} {freq: 0, ln: 0, col: 0}
---- BB4 (RPO 4) (GOTO) (e) LINE 0 (rid_id:0) (flag:7)
Preds:3
Succs:6
Fallthrough: 5
Next : BB5
Prev : BB3
dom_dfs_id(0), dom_dfs_last(0)
pdom_dfs_id(0), pdom_dfs_last(0)
F8F8LDID 77 <st 1793> T<11,.predef_F8,4>
F8STID 75 <st 1793> T<11,.predef_F8,4> {line: 11} {freq: 0, ln: 11, col: 0}
---- BB5 (RPO 5) (Lab2050) (GOTO) (e) LINE 0 (rid_id:0) (flag:7)
Preds:3
Succs:6
Fallthrough: 6
Next : BB6
Prev : BB4
dom_dfs_id(0), dom_dfs_last(0)
pdom_dfs_id(0), pdom_dfs_last(0)
And BB5 is the empty BB with label2050 map.
2010/8/31 Sun Chan <sun...@gm...>
> empty bb is not wrong code. My question is, where did that empty bb come
> from?
> Sun
>
> On Mon, Aug 30, 2010 at 7:21 PM, ruifen Shen <she...@gm...> wrote:
> > 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
> >
> >
>
--
Best Regards.
Shen Ruifen
tel: 010-51266989-226
|