[Cython] prange CEP updated

[mark florisson]

On 4 May 2011 14:23, Dag Sverre Seljebotn <d.s.seljebotn at astro.uio.no> wrote:
> On 05/04/2011 02:17 PM, mark florisson wrote:
>>
>> On 4 May 2011 14:10, Dag Sverre Seljebotn<d.s.seljebotn at astro.uio.no>
>>  wrote:
>>>
>>> On 05/04/2011 01:59 PM, mark florisson wrote:
>>>>
>>>> On 4 May 2011 13:54, Dag Sverre Seljebotn<d.s.seljebotn at astro.uio.no>
>>>>  wrote:
>>>>>
>>>>> On 05/04/2011 01:48 PM, mark florisson wrote:
>>>>>>
>>>>>> On 4 May 2011 13:47, mark florisson<markflorisson88 at gmail.com>
>>>>>>  wrote:
>>>>>>>
>>>>>>> On 4 May 2011 13:45, Dag Sverre Seljebotn<d.s.seljebotn at astro.uio.no>
>>>>>>>  wrote:
>>>>>
>>>>>>>> Look.
>>>>>>>>
>>>>>>>> i = 42
>>>>>>>> for i in prange(n):
>>>>>>>>    f(i)
>>>>>>>> print i # want 42 whenever n == 0
>>>>>>>>
>>>>>>>> Now, translate this to:
>>>>>>>>
>>>>>>>> i = 42;
>>>>>>>> #pragma omp parallel for firstprivate(i) lastprivate(i)
>>>>>>>> for (temp = 0; ...; ...) {
>>>>>>>>    i = ...
>>>>>>>> }
>>>>>>>> #pragma omp parallel end
>>>>>>>> /* At this point, i == 42 if n == 0 */
>>>>>>>>
>>>>>>>> Am I missing something?
>>>>>>>
>>>>>>> Yes, 'i' may be uninitialized with nsteps>      0 (this should be
>>>>>>> valid
>>>>>>> code). So if nsteps>      0, we need to initialize 'i' to something
>>>>>>> to
>>>>>>> get
>>>>>>> correct behaviour with firstprivate.
>>>>>
>>>>> This I don't see. I think I need to be spoon-fed on this one.
>>>>
>>>> So assume this code
>>>>
>>>> cdef int i
>>>>
>>>> for i in prange(10): ...
>>>>
>>>> Now if we transform this without the guard we get
>>>>
>>>> int i;
>>>>
>>>> #pragma omp parallel for firstprivate(i) lastprivate(i)
>>>> for (...) { ...}
>>>>
>>>> This is invalid C code, but valid Cython code. So we need to
>>>> initialize 'i', but then we get our "leave it unaffected for 0
>>>> iterations" paradox. So we need a guard.
>>>
>>> You mean C code won't compile if i is firstprivate and not initialized?
>>> (Sorry, I'm not aware of such things.)
>>
>> It will compile and warn, but it is technically invalid, as you're
>> reading an uninitialized variable, which has undefined behavior. If
>> e.g. the variable contains a trap representation on a certain
>> architecture, it might halt the program (I'm not sure which
>> architecture that would be, but I believe they exist).
>>
>>> My first instinct is to initialize i to 0xbadabada. After all, its value
>>> is
>>> not specified -- we're not violating any Cython specs by initializing it
>>> to
>>> garbage ourselves.
>>
>> The problem is that we don't know whether the user has initialized the
>> variable. So if we want firstprivate to suppress warnings, we should
>> assume that the user hasn't and do it ourselves.
>
> I meant that if we don't care about Valgrindability, we can initialize i at
> the top of our function (i.e. where it says "int __pyx_v_i").

Indeed, but as the current semantics don't do this, I think we also
shouldn't. The good thing is that if we don't do it, the user will see
warnings from the C compiler if used uninitialized.

>>> OTOH, I see that your approach with an if-test is more Valgrind-friendly,
>>> so
>>> I'm OK with that.
>>>
>>> Would it work to do
>>>
>>> if (nsteps>  0) {
>>>    #pragma omp parallel
>>>    i = 0;
>>>    #pragma omp for lastprivate(i)
>>>    for (temp = 0; ...) ...
>>>    ...
>>> }
>>
>> I'm assuming you mean #pragma omp parallel private(i), otherwise you
>> have a race (I'm not sure how much that matters for assignment). In
>> any case, with the private() clause 'i' would be uninitialized
>> afterwards. In either case it won't do anything useful.
>
> Sorry, I meant that lastprivate(i) should go on the parallel line.
>
> if (nsteps>  0) {
>    #pragma omp parallel lastprivate(i)
>    i = 0;
>    #pragma omp for
>    for (temp = 0; ...) ...
>    ...
> }
>
> won't this silence the warning? At any rate, it's obvious you have a better
> handle on this than me, so I'll shut up now and leave you to it :-)

lastprivate() is not valid on a plain parallel constructs, as it's not
a loop. There's only private() and shared().