I was not sure if the *_sync intrinsics required preventing CSE since these intrinsics capture all state as arguments (lanes in a warp to sync as an argument). However, on Volta, I think different lanes in a warp can execute the intrinsic from different syntactic locations (i.e., different program counters). If true, then we do indeed have to model the data exchanged.

Hi Jonas,

Thank you for this. LGTM.

Addressed review comments.

Alexey, do you any more comments on this patch?

Hi Alexey,

Alexey, thank you for your review. I have used SizeTy instead of assuming 64-bits.

Use SizeTy instead of assuming 64 bits!

Hi Alexey,

Minor fixup of comment style on emitInterWarpCopyFunction().

Updated patch to address Alexey's comments. Condensed parameters in emitReduction() to a struct Options.

In D29506#669542, @ABataev wrote:

The patch is too big and quite hard to review? Could you split it into several smaller parts?

Looks like this patch slipped through the cracks :( I've made the requested changes.

Inherit from OMPLexical scope with an added argument to reduce code duplication.

Another correction. We'll have to create a similar scope OMPTeamsScope that inherits from OMPLexicalScope for target-teams combined directives.

The patch was updated to split 'emitParallelOrTeamsOutlinedFunction' into 'emitParallelOutlinedFunction' and 'emitTeamsOutlinedFunction' to enable the use of getCapturedStmt().

Updated 'getOpenMPCaptureRegions' to return the OMPD_teams region kind for the teams directive.

Added a method 'getCapturedStmt' as part of OMPExecutableDirective.

Thanks Alexey.

Use i1 type for bool after all. But this time use the api ConvertType().

Using CGF.ConvertTypeForMem(Context.getBoolType()) to get the right type for 'bool' rather than using i1.

Moved CommonActionTy to CGOpenMPRuntimeNVPTX.cpp and renamed it to NVPTXActionTy, allowing us to customize the class in the future, if necessary.

Updated patch based on reviews.

Alexey, thank you for your review. I've updated the patch addressing your comments.

Alexey and Justin, thank you for spending the time to review this patch. I've updated the patch accordingly. I've also removed a dot ('.') from the worker function name since the character is not accepted by the nvidia linker ptxas in function names.

Addressed comments in review to start function names with a lowercase letter and to fix the enum type name along with the enumerator name.

Stylistic changes to address feedback.

Addressed feedback; see inline comments for details.

Thanks for the quick review! The test cases are the same as the CUDA version so it should be fine.

In D17738#367459, @rafael wrote:

The real stable solution I think is for the IR name to be the final
name. No further mangling involved.

Friendly ping. Rafael, do you have further concerns? Thanks.

In D17738#364717, @rafael wrote:

Wouldn't it be better to error and ask the llvm producer to not create
global values with '@' and '.' when targeting nvptx?

Don't dangle pointers :(

Added convergent to the barrier intrinsics.

Alexey, I've made the change in the comments. Thanks very much for your time!

Committed revision 258832.

Patch fixed.

Addressed fixes by Alexey Bataev. Thanks.

Committed revision 258425.

Added template instantiation test case for all feasible tests.

Added template instantiation test case for all feasible tests.