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
>
>
------------------------------------------------------------------------------
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
