I suggest you include verification somewhere in your fix.
Sun
2010/9/1 Jiangzhou HE <hejiangz...@gmail.com>:
> 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
