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
