Hi, Sun,
Let me state that more clearly.
Currently, lower_mp() uses the verifier to make sure there is no OpenMP
directive after lower_mp() returns (the verifier runs by the destructor).
However, for an OpenMP directive with nested OpenMP directive inside, the
nested OpenMP directive will remain after the outer directive is lowered by
lower_mp(). The verifier will stop the compiler in that case. That is the
bug.
There are two ways to fix the bug. One way is to add new code in lower_mp()
to call itself recursively, so nested OpenMP directives can also be lowered
before the verifier runs. The other solution is to allow the lowered code of
lower_mp() contains OpenMP directives (lower_mp() will be called again by
lower_block(), so finally nested OpenMP directives will also be lowered), so
we need to remove the verifier for lower_mp() (maybe add the same verifier
in some upstream function).
My argument is that it is not appropriate to call lower_mp() itself
recursively, so the second solution is better.
Thanks.
Jiangzhou
2010/9/1 Sun Chan <sun.c...@gmail.com>
> Sorry, I am not familiar with openMP code, but your arguments doesn't
> seem to relate to the verifier. You are saying verifier will call
> lower_mp(), causing that to be recursive?
> Sun
>
> 2010/9/1 Jiangzhou HE <hejiangz...@gmail.com>:
> > Hi, Jianxin,
> >
> > I think there are two reasons why the verifier should be removed from
> > lower_mp().
> >
> > 1. lower_mp() cannot be called recursively because it uses many global
> > variables. Many other functions in that file, which is called by
> lower_mp(),
> > also used these global variables. It is not safe to call lower_mp()
> > recursively. At least, calling lower_mp() recursively is not elegant for
> > such a function which depends on so many global variables.
> >
> > 2. If there is no verifier for lower_mp(), lower_block() will call
> > lower_mp() until all OpenMP directives are lowered. I think that is the
> most
> > natural way to solve the problem (only a verifier removed, no concrete
> code
> > changed). If we solve the problem by calling lower_mp() recursively,
> > lower_block() will do extra work during OMP lowering.
> >
> > How about remove the verifier in lower_mp(), and calling it in some
> upstream
> > function, such as lower_entry()?
> >
> > Thanks.
> >
> > Jiangzhou
> >
> >
> > 2010/9/1 Jian-Xin Lai <laij...@gmail.com>
> >>
> >> I have a question about the item #4 about the Verify_MP_Lowered. I don't
> >> think it's a good fix to remove the verifier. The root issue is there is
> >> only one verifier and the pointer is passed so that it can not be used
> >> recursively.
> >>
> >> 2010/8/31 Jiangzhou HE <hejiangz...@gmail.com>
> >>>
> >>> Hi, all,
> >>>
> >>> This is the fixes for several OpenMP bugs. Could gatekeeper help to
> >>> review it? I attached the test case for the fix.
> >>>
> >>> I have fixed the following bugs:
> >>>
> >>> 1. bug of driver (see test_has_openmp.c)
> >>> The case works with -mp but fails with -fopenmp. Since the command
> >>> line options should be compatible with gcc, -fopenmp should also work.
> >>> I fixed this bug by change osprey/driver/OPTION.
> >>>
> >>> 2. bug of atomic directive (see test_atomic.c)
> >>> Atomic directive did not work correctly sometimes.
> >>> Without this fix, atomic directive was lowered to
> >>> INTRN_COMPARE_AND_SWAP_XX. The lowered code checks whether its result
> is
> >>> non-zero, to decide whether to try again. So INTRN_COMPARE_AND_SWAP_XX
> >>> should be replaced by INTRN_BOOL_COMPARE_AND_SWAP in the lowered code.
> >>> The change for omp_lower.cxx is for this bug.
> >>>
> >>> 3. bug of variable declared in nested block (see test_local_var.c)
> >>>
> >>> For code like this:
> >>>
> >>> int s;
> >>> #pragma parallel
> >>> {
> >>> int t;
> >>> ....
> >>> }
> >>>
> >>> In the parallel region, the variable s should be shared by default,
> >>> and the variable t should be private. However, in WHIRL, all the local
> >>> variables are put in the symbol table of current function, so the
> declaring
> >>> block for local variables such as s and t cannot be distinguished.
> >>> Therefore, in the backend, both s and t are treated as shared variable,
> >>> which is not correct.
> >>> I fixed this bug by adding some code in wgen. By this fix, wgen
> will
> >>> insert an explicit private pragma for local variables such as t.
> >>>
> >>> 4. bug of nested OpenMP directives (see test_nested_critical.c)
> >>>
> >>> Some OpenMP directives, like critical, master, can be nested with
> each
> >>> other.
> >>>
> >>> Most OpenMP directives are lowered by lower_mp() (in
> >>> osprey/be/com/wn_mp.cxx). lower_mp() is called by lower_block() (in
> >>> osprey/be/com/wn_lower.cxx). lower_mp() returns the first statement of
> the
> >>> replacing block, so lower_block() will call lower_mp() again for the
> nested
> >>> directives. I believe that is the right way to deal with the nested
> OpenMP
> >>> directives ( another way may be run lower_mp() recursively, however,
> since
> >>> lower_mp() uses many global variables, I think it was not designed to
> be
> >>> called recursively).
> >>>
> >>> However, there is a class Verify_MP_Lowered in wn_mp.cxx to check
> >>> whether all the generated code in lower_mp() has not OpenMP directives.
> >>> Since for nested OpenMP directives, the generated code in the first
> >>> invocation of lower_mp() would certainly contain OpenMP directives
> (which
> >>> will be lowered in the next invocation of lower_mp()), I think we
> should
> >>> remove that verifier here. Most changes of wn_mp.cxx is for that.
> >>>
> >>>
> >>> After applying the fixes, all the cases work.
> >>>
> >>> Thanks.
> >>>
> >>> Regards,
> >>> Jiangzhou
> >>>
> >>> --
> >>> Jiangzhou HE (何江舟)
> >>>
> >>> Institute of High-Performance Computing
> >>> Department of Computer Science and Technology
> >>> Tsinghua University, Beijing, China
> >>>
> >>>
> >>>
> >>>
> ------------------------------------------------------------------------------
> >>> This SF.net Dev2Dev email is sponsored by:
> >>>
> >>> Show off your parallel programming skills.
> >>> Enter the Intel(R) Threading Challenge 2010.
> >>> http://p.sf.net/sfu/intel-thread-sfd
> >>> _______________________________________________
> >>> Open64-devel mailing list
> >>> Open64-devel@lists.sourceforge.net
> >>> https://lists.sourceforge.net/lists/listinfo/open64-devel
> >>>
> >>
> >>
> >>
> >> --
> >> Regards,
> >> Lai Jian-Xin
> >
> >
> >
> > --
> > Jiangzhou HE (何江舟)
> >
> > Institute of High-Performance Computing
> > Department of Computer Science and Technology
> > Tsinghua University, Beijing, China
> >
> >
> >
> ------------------------------------------------------------------------------
> > This SF.net Dev2Dev email is sponsored by:
> >
> > Show off your parallel programming skills.
> > Enter the Intel(R) Threading Challenge 2010.
> > http://p.sf.net/sfu/intel-thread-sfd
> > _______________________________________________
> > Open64-devel mailing list
> > Open64-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/open64-devel
> >
> >
>
--
Jiangzhou HE (何江舟)
Institute of High-Performance Computing
Department of Computer Science and Technology
Tsinghua University, Beijing, China
------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
