Skip to content

Commit

Permalink
[OpenMP][OMPT] Fix reported target pointer for data alloc callback
Browse files Browse the repository at this point in the history 
This patch fixes: #64671
DataOp EMI callbacks would not report the correct target pointer.
This is now alleviated by passing a `void**` into the function which
emits the actual callback, then evaluating that pointer.

Note: Since this is only done after the pointer has been properly
updated, only `endpoint=2` callbacks will show a non-null value.

Reviewed By: dhruvachak, jdoerfert

Differential Revision: https://reviews.llvm.org/D157996
  • Loading branch information
@mhalk
mhalk committed Aug 16, 2023
1 parent 2459ed6 commit 41f3626
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 11 deletions.
12 changes: 7 additions & 5 deletions openmp/libomptarget/src/OmptCallback.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,37 +71,39 @@ static uint64_t createRegionId() {
}

void Interface::beginTargetDataAlloc(int64_t DeviceId, void *HstPtrBegin,
size_t Size, void *Code) {
void **TgtPtrBegin, size_t Size,
void *Code) {
beginTargetDataOperation();
if (ompt_callback_target_data_op_emi_fn) {
// HostOpId will be set by the tool. Invoke the tool supplied data op EMI
// callback
ompt_callback_target_data_op_emi_fn(
ompt_scope_begin, TargetTaskData, &TargetData, &TargetRegionOpId,
ompt_target_data_alloc, HstPtrBegin,
/* SrcDeviceNum */ omp_get_initial_device(), /* TgtPtrBegin */ nullptr,
/* SrcDeviceNum */ omp_get_initial_device(), *TgtPtrBegin,
/* TgtDeviceNum */ DeviceId, Size, Code);
} else if (ompt_callback_target_data_op_fn) {
// HostOpId is set by the runtime
HostOpId = createOpId();
// Invoke the tool supplied data op callback
ompt_callback_target_data_op_fn(
TargetData.value, HostOpId, ompt_target_data_alloc, HstPtrBegin,
/* SrcDeviceNum */ omp_get_initial_device(), /* TgtPtrBegin */ nullptr,
/* SrcDeviceNum */ omp_get_initial_device(), *TgtPtrBegin,
/* TgtDeviceNum */ DeviceId, Size, Code);
}
}

void Interface::endTargetDataAlloc(int64_t DeviceId, void *HstPtrBegin,
size_t Size, void *Code) {
void **TgtPtrBegin, size_t Size,
void *Code) {
// Only EMI callback handles end scope
if (ompt_callback_target_data_op_emi_fn) {
// HostOpId will be set by the tool. Invoke the tool supplied data op EMI
// callback
ompt_callback_target_data_op_emi_fn(
ompt_scope_end, TargetTaskData, &TargetData, &TargetRegionOpId,
ompt_target_data_alloc, HstPtrBegin,
/* SrcDeviceNum */ omp_get_initial_device(), /* TgtPtrBegin */ nullptr,
/* SrcDeviceNum */ omp_get_initial_device(), *TgtPtrBegin,
/* TgtDeviceNum */ DeviceId, Size, Code);
}
endTargetDataOperation();
Expand Down
8 changes: 4 additions & 4 deletions openmp/libomptarget/src/OmptInterface.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ static ompt_get_target_task_data_t ompt_get_target_task_data_fn;
class Interface {
public:
/// Top-level function for invoking callback before device data allocation
void beginTargetDataAlloc(int64_t DeviceId, void *TgtPtrBegin, size_t Size,
void *Code);
void beginTargetDataAlloc(int64_t DeviceId, void *HstPtrBegin,
void **TgtPtrBegin, size_t Size, void *Code);

/// Top-level function for invoking callback after device data allocation
void endTargetDataAlloc(int64_t DeviceId, void *TgtPtrBegin, size_t Size,
void *Code);
void endTargetDataAlloc(int64_t DeviceId, void *HstPtrBegin,
void **TgtPtrBegin, size_t Size, void *Code);

/// Top-level function for invoking callback before data submit
void beginTargetDataSubmit(int64_t DeviceId, void *HstPtrBegin,
Expand Down
6 changes: 4 additions & 2 deletions openmp/libomptarget/src/device.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,12 +580,14 @@ __tgt_target_table *DeviceTy::loadBinary(void *Img) {

void *DeviceTy::allocData(int64_t Size, void *HstPtr, int32_t Kind) {
/// RAII to establish tool anchors before and after data allocation
void *TargetPtr = nullptr;
OMPT_IF_BUILT(InterfaceRAII TargetDataAllocRAII(
RegionInterface.getCallbacks<ompt_target_data_alloc>(),
RTLDeviceID, HstPtr, Size,
RTLDeviceID, HstPtr, &TargetPtr, Size,
/* CodePtr */ OMPT_GET_RETURN_ADDRESS(0));)

return RTL->data_alloc(RTLDeviceID, Size, HstPtr, Kind);
TargetPtr = RTL->data_alloc(RTLDeviceID, Size, HstPtr, Kind);
return TargetPtr;
}

int32_t DeviceTy::deleteData(void *TgtAllocBegin, int32_t Kind) {
Expand Down
4 changes: 4 additions & 0 deletions openmp/libomptarget/test/ompt/veccopy_disallow_both.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ int main() {
/// CHECK: Callback Target EMI: kind=1 endpoint=1
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback Submit: target_id=[[TARGET_ID:[0-9]+]] host_op_id=[[HOST_OP_ID:[0-9]+]] req_num_teams=1
Expand All @@ -82,10 +84,12 @@ int main() {
/// CHECK: Callback Target EMI: kind=1 endpoint=1
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback Submit: target_id=[[TARGET_ID:[0-9]+]] host_op_id=[[HOST_OP_ID:[0-9]+]] req_num_teams=0
Expand Down
4 changes: 4 additions & 0 deletions openmp/libomptarget/test/ompt/veccopy_emi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,12 @@ int main() {
/// CHECK: Callback Target EMI: kind=1 endpoint=1
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback Submit EMI: endpoint=1 req_num_teams=1
Expand All @@ -81,10 +83,12 @@ int main() {
/// CHECK: Callback Target EMI: kind=1 endpoint=1
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback Submit EMI: endpoint=1 req_num_teams=0
Expand Down
4 changes: 4 additions & 0 deletions openmp/libomptarget/test/ompt/veccopy_emi_map.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,12 @@ int main() {
/// CHECK: Callback Target EMI: kind=1 endpoint=1
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback Submit EMI: endpoint=1 req_num_teams=1
Expand All @@ -82,10 +84,12 @@ int main() {
/// CHECK: Callback Target EMI: kind=1 endpoint=1
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback DataOp EMI: endpoint=1 optype=1
/// CHECK: Callback DataOp EMI: endpoint=2 optype=1
/// CHECK-NOT: dest=(nil)
/// CHECK: Callback DataOp EMI: endpoint=1 optype=2
/// CHECK: Callback DataOp EMI: endpoint=2 optype=2
/// CHECK: Callback Submit EMI: endpoint=1 req_num_teams=0
Expand Down

0 comments on commit 41f3626

Please sign in to comment.