Re: [Open64-devel] Code review to re-patch the bug #897 [WOPT]

[Sun C. <sun...@gm...>]

can you go over why the second change, set_BB(null)? And remove the
mark Reset_live_stmt()?
Sun

On Thu, Dec 22, 2011 at 3:54 PM, Jian-Xin Lai <la...@gm...> wrote:
> Hi,
>
> Here is a smaller case for bug #897 and a new patch based on Gang's
> current work. Could a gatekeeper review it? Thank you very much.
>
> A smaller case:
>  1  int g_1;
>  2  int* g_2;
>  3  int* func(int* p_1, short p_2) {
>  4      int **l_1;
>  5      const unsigned long l_2 = 0x2A1DFCF9L;
>  6      for ( ; ; p_2 = foo() ) {
>  7          int ***l_3 = &l_1;
>  8          (*l_3) = &g_2;
>  9          if (!l_2) {
> 10              // The following code is unreachable
> 11              if ((bar())) {
> 12                  (*g_2) |= g_1;
> 13                  (**l_1) |= (*p_1);
> 14                  g_2 = &g_1;
> 15              }
> 16          }
> 17      }
> 18  }
>
> in DCE phase, the unreachable code elimination runs at first and all
> the statements in the if block is removed and set to NOT_LIVE.  In the
> later Dead store elimination phase, as Gang's explaination, we follow
> the U-D chain of g_2, which is a global variable and has CHI on line
> 6(foo), 11(bar), 12 (*g_2) and a real dec in line 14. When we mark the
> call of foo (line 6) to be LIVE, the CHI of g_2 is also LIVE and
> following the U-D chain, the CHI's on bar() and (*g_2) is also marked
> to LIVE. When marking (*g_2) to live, we also mark that BB to live.
> But the BB is empty (all statements have been removed in earlier
> phase) and the assertion occures.
>
> Here is a new patch:
> Index: osprey/be/opt/opt_dce.cxx
> ===================================================================
> --- osprey/be/opt/opt_dce.cxx   (revision 3855)
> +++ osprey/be/opt/opt_dce.cxx   (working copy)
> @@ -2619,7 +2619,10 @@
>       // NOTE: May at some time always have a defining statement, so
>       //       we could remove the check for a null defstmt
>       //         (i.e., if live-in values get some dummy def stmt)
> -       if ( cr->Defstmt() != NULL && !cr->Defstmt()->Live_stmt() ) {
> +        // if defining statement doesn't belong to any BB, it's been
> +        // removed in early unreachable code elimination phase.
> +       if ( cr->Defstmt() != NULL && !cr->Defstmt()->Live_stmt() &&
> +             cr->Defstmt()->Bb() != NULL ) {
>         // if we're making this statement live just because it's
>         // associated with a chi, see if we can get rid of "i = i"
>         // assignments.  This only happens when zero-version is
> Index: osprey/be/opt/opt_bb.cxx
> ===================================================================
> --- osprey/be/opt/opt_bb.cxx    (revision 3855)
> +++ osprey/be/opt/opt_bb.cxx    (working copy)
> @@ -734,6 +734,7 @@
>
>  _stmtlist.Remove(stmt);
> -  stmt->Reset_live_stmt();  // WHIRL SSA: mark stmt dead
> +  stmt->Set_bb(NULL);
>  }
>
>  void
>
> If the statement has been removed from the BB, we reset its BB to
> NULL. In the later phase, if a statement's BB is NULL, which means the
> statement has been removed and we don't need to mark it to live.
> Reseting the bb of dead statement to NULL also helps preventing more
> bugs when trying to traverse the BB.
>
> --
> Regards,
> Lai Jian-Xin
>
> ------------------------------------------------------------------------------
> Write once. Port to many.
> Get the SDK and tools to simplify cross-platform app development. Create
> new or port existing apps to sell to consumers worldwide. Explore the
> Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
> http://p.sf.net/sfu/intel-appdev
> _______________________________________________
> Open64-devel mailing list
> Ope...@li...
> https://lists.sourceforge.net/lists/listinfo/open64-devel