https://github.com/koparasy commented:
Overall, this PR looks good. I am new to the omp dialect. A couple of thoughts
on our handling here on the overall design of this handling (it exceeds what
you do in this PR).
`EHABILowering: lowerInitCatchParam` creates `exn.byref.tmp` at the start of
`getParentOfType<cir::FuncOp>().getBody().front()`. If the catch is inside an
`omp.parallel`, I believe that `alloca` ends up outside the region.
`AFAIU` it becomes a value captured from the enclosing function, so all threads
share one stack slot, which is a data race. `FlattenCFG:
getOrCreateCleanupDestSlot` does the same thing with `__cleanup_dest_slot`,
which is one slot per function. A cleanup scope with more than one exit inside
a parallel region would use that shared slot from every thread. An example is a
local with a non-trivial destructor in a loop body that has a break or
continue.
I think something like this below will result to the race:
```cpp
struct S {
S();
~S();
};
bool cond(int);
void cleanup_dest_in_parallel() {
#pragma omp parallel
{
while (cond(0)) {
S s;
if (cond(1))
break;
if (cond(2))
continue;
}
}
}
```
I think both issues come from pre-existing functionality. So, I am ok with this
being merged, but if this is the direction you go, please create an issue so
that we can track it. Thanks!
https://github.com/llvm/llvm-project/pull/230509
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
