[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime
AlexEichenberger requested changes to this revision. AlexEichenberger added a comment. This revision now requires changes to proceed. see note above; apologies if it is already done and hiding somewhere I did not see Comment at: lib/CodeGen/CGOpenMPRuntime.h:641 + /// directive is present. + bool HasRequiresUnifiedSharedMemory = false; + Don't you need a bool for each characteristics? Your intention is to have one bit vector for each characteristics that matter to the compiler? Also, it is my belief that you need to record 2 states: unmaterialized (meaning I have not processed any target yet, so I should record any requires as they arrive) finalized (I am processing a target, so the state is now fixed) you need this in case you have an input like this: declare target int x end declare target requires unified memory which is illegal Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime
AlexEichenberger accepted this revision. AlexEichenberger added a comment. This revision is now accepted and ready to land. fantastic Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63009: [OpenMP] Add target task alloc function with device ID
AlexEichenberger added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:610 + // size_t sizeof_shareds, kmp_routine_entry_t *task_entry, + // size_t device_id); + OMPRTL__kmpc_omp_target_task_alloc, device_id is int64_t Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1928 +CGM.SizeTy, CGM.SizeTy, KmpRoutineEntryPtrTy, +CGM.SizeTy}; +// Return void * and then cast to particular kmp_task_t type. device_id must be a int64_t type, not a machine dependent quantity. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63009/new/ https://reviews.llvm.org/D63009 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63010: [OpenMP] Add task alloc function
AlexEichenberger accepted this revision. AlexEichenberger added a comment. This revision is now accepted and ready to land. LGTM Repository: rOMP OpenMP CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63010/new/ https://reviews.llvm.org/D63010 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63009: [OpenMP] Add target task alloc function with device ID
AlexEichenberger accepted this revision. AlexEichenberger added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63009/new/ https://reviews.llvm.org/D63009 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63009: [OpenMP] Add target task alloc function with device ID
AlexEichenberger added inline comments.
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:5122
+NewTask = CGF.EmitRuntimeCall(
+ createRuntimeFunction(OMPRTL__kmpc_omp_target_task_alloc), AllocArgs);
+ } else {
gtbercea wrote:
> ABataev wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > gtbercea wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Can we use the same function in both modes, with nowait clause
> > > > > > > > and without?
> > > > > > > For nowait we need to use the target_task_alloc variant. There
> > > > > > > are runtimes - other than the open source one - for which this
> > > > > > > function does something different.
> > > > > > Trunk relies only to libomptarget interfaces. Why we should take
> > > > > > into account some other runtime libraries? Plus, what's so
> > > > > > different for async and non-async versions of the directive?
> > > > > Async is not yet fully supported in the OpenMP open source runtime
> > > > > but at some point it will be. This is the first step towards its
> > > > > support. I'm not sure what your objection is. The difference is
> > > > > clear. Device ID must be passed on the async version of this call.
> > > > The difference is not obvious. Why we can't use the same function for
> > > > sync directives? The fact the it has DeviceId parameter is not an
> > > > argument here. What's so special from functional point of view?
> > > When you have multiple device on the same system you have to be able to
> > > distinguish between then and manage dependencies across these different
> > > devices. Knowing the device is a crucial first step in handling these
> > > inter-device dependencies.
> > Ok, this is why we need it for async version. But why we can't use for sync
> > version?
> Based on how the runtime is currently structured, nowait functions are
> separate from the synchronous ones. See target vs. target_nowait functions
> and there are many examples like that. The async support is always kept
> separate from the synchronous support at least in terms of entry points in
> the runtime.
Right now, target_task indicates two things: 1) there are depenences to offload
and 2) it's a nowait constructs. If we use target_task_alloc for both
situations, we need to distinguish these two cases using flags, because the
handling iseventually different.
If we want to explore this avenue, then we have to look into using the proxy
flag is set to indicates nowait. It may have implications for the tasks on the
KMP side, so this might be more risky.
This is why I suggest at the present time that we only generate target task for
the limited "target task depend nowait"
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63009/new/
https://reviews.llvm.org/D63009
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
