sorry, please go ahead
Sun
2010/9/19 Jiangzhou HE <hejiangz...@gmail.com>:
> Can I check in the patch now? Thanks.
>
> 2010/9/2 Jiangzhou HE <hejiangz...@gmail.com>
>>
>> I'm sorry, there is a minor problem in the patch which I submitted five
>> hours ago: Verify_No_MP() should be called for the body of a function, not
>> the whole function entry, since some other kids of a function entry may
>> contain OpenMP related pragmas after lowering. I have changed that and
>> tested that carefully.
>>
>> Thanks.
>>
>> Jiangzhou
>>
>> 2010/9/2 Jiangzhou HE <hejiangz...@gmail.com>
>>>
>>> OK. I have added the verifier at lower_entry() and
>>> Process_Parallel_Region(), which will cover the whole program. I have
>>> attached the new patch.
>>>
>>> Thanks.
>>>
>>> Jiangzhou
>>>
>>> 2010/9/1 Sun Chan <sun.c...@gmail.com>
>>>>
>>>> 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
>>>> >
>>>> >
>>>
>>>
>>>
>>> --
>>> Jiangzhou HE (何江舟)
>>>
>>> Institute of High-Performance Computing
>>> Department of Computer Science and Technology
>>> Tsinghua University, Beijing, China
>>>
>>
>>
>>
>> --
>> Jiangzhou HE (何江舟)
>>
>> Institute of High-Performance Computing
>> Department of Computer Science and Technology
>> Tsinghua University, Beijing, China
>>
>
>
>
> --
> Jiangzhou HE (何江舟)
>
> Institute of High-Performance Computing
> Department of Computer Science and Technology
> Tsinghua University, Beijing, China
>
>
> ------------------------------------------------------------------------------
> Start uncovering the many advantages of virtual appliances
> and start using them to simplify application deployment and
> accelerate your shift to cloud computing.
> http://p.sf.net/sfu/novell-sfdev2dev
> _______________________________________________
> Open64-devel mailing list
> Open64-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/open64-devel
>
>
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
