Sync before exiting a cilk_scope with a non-compound statement body #320
Conversation
VoxSciurorum
force-pushed
the
jfc/scope
branch
from
7554f75 to
101b84a
Compare
clang/lib/CodeGen/CGCilk.cpp
Outdated
| EmitStmt(S.getBody()); | ||
| EmitStmt(S.getBody()); | ||
| } else { | ||
| PushSyncRegion()->addImplicitSync(); |
There was a problem hiding this comment.
It took me a while to figure out why these changes were necessary, given that the SyncRegionRAII is meant to handle pushing and popping of sync regions.
I think the simpler approach to resolving this issue is to set ScopeIsSynced to true before pushing the TapirRuntimeEndCleanup, e.g., on line 472. That flag is used to ensure that implicit syncs and cleanups/destructors are ordered correctly within a lexical scope. Does that approach work for your tests?
|
The second version crashes on this input, processed by the first version: void f()
{
extern void function(void);
cilk_scope { cilk_scope cilk_spawn { function(); } }
} |
|
I see. It looks like we also need to ensure the Does it work if we move the |
|
Moving the assignment earlier worked. I added another test for nested |
Fixes #319.