Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

memcheck - clarify vector access #1664

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

memcheck - clarify vector access #1664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c63ecbc
memcheck - clarify vector access
jeremylt Sep 16, 2024
31020a9
memcheck - minor clarify on QF impl
jeremylt Sep 16, 2024
91b5d27
memcheck - also tidy up the ctx impl
jeremylt Sep 16, 2024
7e5ca22
memcheck - emulate device vector methods too
jeremylt Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions backends/memcheck/ceed-memcheck-qfunction.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
static int CeedQFunctionApply_Memcheck(CeedQFunction qf, CeedInt Q, CeedVector *U, CeedVector *V) {
Ceed ceed;
void *ctx_data = NULL;
int input_block_ids[CEED_FIELD_MAX], output_block_ids[CEED_FIELD_MAX];
CeedInt num_in, num_out;
CeedQFunctionUser f = NULL;
CeedQFunctionField *output_fields;
Expand All @@ -29,12 +30,21 @@ static int CeedQFunctionApply_Memcheck(CeedQFunction qf, CeedInt Q, CeedVector *
CeedCallBackend(CeedQFunctionGetContextData(qf, CEED_MEM_HOST, &ctx_data));
CeedCallBackend(CeedQFunctionGetUserFunction(qf, &f));
CeedCallBackend(CeedQFunctionGetNumArgs(qf, &num_in, &num_out));
int mem_block_ids[num_out];

// Get input/output arrays
// Get input arrays
for (CeedInt i = 0; i < num_in; i++) {
CeedSize len;
char name[32] = "";

CeedCallBackend(CeedVectorGetArrayRead(U[i], CEED_MEM_HOST, &impl->inputs[i]));

CeedCallBackend(CeedVectorGetLength(U[i], &len));

snprintf(name, 32, "QFunction input %" CeedInt_FMT, i);
input_block_ids[i] = VALGRIND_CREATE_BLOCK(impl->inputs[i], len, name);
}

// Get output arrays
for (CeedInt i = 0; i < num_out; i++) {
CeedSize len;
char name[32] = "";
Expand All @@ -44,8 +54,8 @@ static int CeedQFunctionApply_Memcheck(CeedQFunction qf, CeedInt Q, CeedVector *
CeedCallBackend(CeedVectorGetLength(V[i], &len));
VALGRIND_MAKE_MEM_UNDEFINED(impl->outputs[i], len);

snprintf(name, 32, "'QFunction output %" CeedInt_FMT "'", i);
mem_block_ids[i] = VALGRIND_CREATE_BLOCK(impl->outputs[i], len, name);
snprintf(name, 32, "QFunction output %" CeedInt_FMT, i);
output_block_ids[i] = VALGRIND_CREATE_BLOCK(impl->outputs[i], len, name);
}

// Call user function
Expand All @@ -54,8 +64,10 @@ static int CeedQFunctionApply_Memcheck(CeedQFunction qf, CeedInt Q, CeedVector *
// Restore input arrays
for (CeedInt i = 0; i < num_in; i++) {
CeedCallBackend(CeedVectorRestoreArrayRead(U[i], &impl->inputs[i]));
VALGRIND_DISCARD(input_block_ids[i]);
}
// Check for unset output values

// Check for unset output values and restore arrays
{
const char *kernel_name, *kernel_path;

Expand All @@ -73,7 +85,7 @@ static int CeedQFunctionApply_Memcheck(CeedQFunction qf, CeedInt Q, CeedVector *
kernel_name);
}
CeedCallBackend(CeedVectorRestoreArray(V[i], &impl->outputs[i]));
VALGRIND_DISCARD(mem_block_ids[i]);
VALGRIND_DISCARD(output_block_ids[i]);
}
}
CeedCallBackend(CeedQFunctionRestoreContextData(qf, &ctx_data));
Expand Down
175 changes: 136 additions & 39 deletions backends/memcheck/ceed-memcheck-qfunctioncontext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ static int CeedQFunctionContextHasValidData_Memcheck(CeedQFunctionContext ctx, b
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
*has_valid_data = impl->data;
*has_valid_data = !!impl->data_allocated;
return CEED_ERROR_SUCCESS;
}

Expand All @@ -30,9 +30,10 @@ static int CeedQFunctionContextHasValidData_Memcheck(CeedQFunctionContext ctx, b
static int CeedQFunctionContextHasBorrowedDataOfType_Memcheck(CeedQFunctionContext ctx, CeedMemType mem_type, bool *has_borrowed_data_of_type) {
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");
*has_borrowed_data_of_type = impl->data_borrowed;

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
*has_borrowed_data_of_type = !!impl->data_borrowed;
return CEED_ERROR_SUCCESS;
}

Expand All @@ -43,52 +44,97 @@ static int CeedQFunctionContextSetData_Memcheck(CeedQFunctionContext ctx, CeedMe
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

// Clear previous owned data buffers
if (impl->data_allocated) {
memset(impl->data_allocated, -42, ctx_size);
VALGRIND_DISCARD(impl->allocated_block_id);
}
CeedCallBackend(CeedFree(&impl->data_allocated));
if (impl->data_owned) {
memset(impl->data_owned, -42, ctx_size);
VALGRIND_DISCARD(impl->owned_block_id);
}
CeedCallBackend(CeedFree(&impl->data_owned));

// Clear borrowed block id, if present
if (impl->data_borrowed) VALGRIND_DISCARD(impl->borrowed_block_id);

// Set internal pointers to external buffers
switch (copy_mode) {
case CEED_COPY_VALUES:
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_owned));
impl->data_owned = NULL;
impl->data_borrowed = NULL;
impl->data = impl->data_owned;
memcpy(impl->data, data, ctx_size);
break;
case CEED_OWN_POINTER:
impl->data_owned = data;
impl->data_borrowed = NULL;
impl->data = data;
impl->data_owned = data;
impl->data_borrowed = NULL;
impl->owned_block_id = VALGRIND_CREATE_BLOCK(impl->data_owned, ctx_size, "Owned external data buffer");
break;
case CEED_USE_POINTER:
impl->data_borrowed = data;
impl->data = data;
impl->data_owned = NULL;
impl->data_borrowed = data;
impl->owned_block_id = VALGRIND_CREATE_BLOCK(impl->data_borrowed, ctx_size, "Borrowed external data buffer");
}
// Copy data to check ctx_size bounds

// Create internal data buffer
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_allocated));
memcpy(impl->data_allocated, impl->data, ctx_size);
impl->data = impl->data_allocated;
VALGRIND_DISCARD(impl->mem_block_id);
impl->mem_block_id = VALGRIND_CREATE_BLOCK(impl->data, ctx_size, "'QFunction backend context data copy'");
impl->allocated_block_id = VALGRIND_CREATE_BLOCK(impl->data_allocated, ctx_size, "'Allocated internal context data buffer");
memcpy(impl->data_allocated, data, ctx_size);
return CEED_ERROR_SUCCESS;
}

//------------------------------------------------------------------------------
// Sync data
//------------------------------------------------------------------------------
static int CeedQFunctionContextSyncData_Memcheck(CeedQFunctionContext ctx, CeedMemType mem_type) {
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));

// Copy internal buffer back to owned or borrowed data buffer
if (impl->data_owned) {
memcpy(impl->data_owned, impl->data_allocated, ctx_size);
}
if (impl->data_borrowed) {
memcpy(impl->data_borrowed, impl->data_allocated, ctx_size);
}
return CEED_ERROR_SUCCESS;
}

//------------------------------------------------------------------------------
// QFunctionContext Take Data
//------------------------------------------------------------------------------
static int CeedQFunctionContextTakeData_Memcheck(CeedQFunctionContext ctx, CeedMemType mem_type, void *data) {
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only provide HOST memory for this backend");

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only provide HOST memory for this backend");
// Synchronize memory
CeedCallBackend(CeedQFunctionContextSyncData_Memcheck(ctx, CEED_MEM_HOST));

// Return borrowed buffer
*(void **)data = impl->data_borrowed;
impl->data_borrowed = NULL;
impl->data = NULL;
VALGRIND_DISCARD(impl->mem_block_id);
VALGRIND_DISCARD(impl->borrowed_block_id);

// De-allocate internal memory
if (impl->data_allocated) {
memset(impl->data_allocated, -42, ctx_size);
VALGRIND_DISCARD(impl->allocated_block_id);
}
CeedCallBackend(CeedFree(&impl->data_allocated));
return CEED_ERROR_SUCCESS;
}
Expand All @@ -97,13 +143,19 @@ static int CeedQFunctionContextTakeData_Memcheck(CeedQFunctionContext ctx, CeedM
// QFunctionContext Get Data
//------------------------------------------------------------------------------
static int CeedQFunctionContextGetData_Memcheck(CeedQFunctionContext ctx, CeedMemType mem_type, void *data) {
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only provide HOST memory for this backend");
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));

*(void **)data = impl->data;
// Create and return writable buffer
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_writable_copy));
impl->writable_block_id = VALGRIND_CREATE_BLOCK(impl->data_writable_copy, ctx_size, "Allocated writeable data buffer copy");
memcpy(impl->data_writable_copy, impl->data_allocated, ctx_size);
*(void **)data = impl->data_writable_copy;
return CEED_ERROR_SUCCESS;
}

Expand All @@ -114,13 +166,18 @@ static int CeedQFunctionContextGetDataRead_Memcheck(CeedQFunctionContext ctx, Ce
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));
CeedCallBackend(CeedQFunctionContextGetData_Memcheck(ctx, mem_type, data));

// Make copy to verify no write occurred
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_read_only_copy));
memcpy(impl->data_read_only_copy, *(void **)data, ctx_size);
// Create and return read-only buffer
if (!impl->data_read_only_copy) {
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_read_only_copy));
impl->writable_block_id = VALGRIND_CREATE_BLOCK(impl->data_read_only_copy, ctx_size, "Allocated read-only data buffer copy");
memcpy(impl->data_read_only_copy, impl->data_allocated, ctx_size);
}
*(void **)data = impl->data_read_only_copy;
return CEED_ERROR_SUCCESS;
}

Expand All @@ -134,46 +191,76 @@ static int CeedQFunctionContextRestoreData_Memcheck(CeedQFunctionContext ctx) {
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));

if (impl->data_borrowed) memcpy(impl->data_borrowed, impl->data, ctx_size);
if (impl->data_owned) memcpy(impl->data_owned, impl->data, ctx_size);
// Copy back to internal buffer and sync
memcpy(impl->data_allocated, impl->data_writable_copy, ctx_size);
CeedCallBackend(CeedQFunctionContextSyncData_Memcheck(ctx, CEED_MEM_HOST));

// Invalidate writable buffer
memset(impl->data_writable_copy, -42, ctx_size);
CeedCallBackend(CeedFree(&impl->data_writable_copy));
VALGRIND_DISCARD(impl->writable_block_id);
return CEED_ERROR_SUCCESS;
}

//------------------------------------------------------------------------------
// QFunctionContext Restore Data Read-Only
//------------------------------------------------------------------------------
static int CeedQFunctionContextRestoreDataRead_Memcheck(CeedQFunctionContext ctx) {
Ceed ceed;
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetCeed(ctx, &ceed));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));

CeedCheck(!memcmp(impl->data, impl->data_read_only_copy, ctx_size), CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND,
"Context data changed while accessed in read-only mode");
// Verify no changes made during read-only access
bool is_changed = memcmp(impl->data_allocated, impl->data_read_only_copy, ctx_size);

CeedCheck(!is_changed, ceed, CEED_ERROR_BACKEND, "Context data changed while accessed in read-only mode");

// Invalidate read-only buffer
memset(impl->data_read_only_copy, -42, ctx_size);
CeedCallBackend(CeedFree(&impl->data_read_only_copy));
VALGRIND_DISCARD(impl->read_only_block_id);
return CEED_ERROR_SUCCESS;
}

//------------------------------------------------------------------------------
// QFunctionContext destroy user data
//------------------------------------------------------------------------------
static int CeedQFunctionContextDataDestroy_Memcheck(CeedQFunctionContext ctx) {
Ceed ceed;
CeedMemType data_destroy_mem_type;
CeedQFunctionContextDataDestroyUser data_destroy_function;
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetCeed(ctx, &ceed));
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetDataDestroy(ctx, &data_destroy_mem_type, &data_destroy_function));

CeedCheck(data_destroy_mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND,
"Can only destroy HOST memory for this backend");
CeedCallBackend(CeedQFunctionContextGetDataDestroy(ctx, &data_destroy_mem_type, &data_destroy_function));
CeedCheck(data_destroy_mem_type == CEED_MEM_HOST, ceed, CEED_ERROR_BACKEND, "Can only destroy HOST memory for this backend");

// Run user destroy routine
if (data_destroy_function) {
CeedCallBackend(data_destroy_function(impl->data_borrowed ? impl->data_borrowed : impl->data_owned));
bool is_borrowed = !!impl->data_borrowed;

CeedCallBackend(data_destroy_function(is_borrowed ? impl->data_borrowed : impl->data_owned));
if (is_borrowed) VALGRIND_DISCARD(impl->borrowed_block_id);
else VALGRIND_DISCARD(impl->owned_block_id);
}
// Free allocations and discard block ids
if (impl->data_allocated) {
CeedCallBackend(CeedFree(&impl->data_allocated));
VALGRIND_DISCARD(impl->allocated_block_id);
}
if (impl->data_owned) {
CeedCallBackend(CeedFree(&impl->data_owned));
VALGRIND_DISCARD(impl->owned_block_id);
}
if (impl->data_borrowed) {
VALGRIND_DISCARD(impl->borrowed_block_id);
}
CeedCallBackend(CeedFree(&impl->data_allocated));
return CEED_ERROR_SUCCESS;
}

Expand All @@ -183,9 +270,19 @@ static int CeedQFunctionContextDataDestroy_Memcheck(CeedQFunctionContext ctx) {
static int CeedQFunctionContextDestroy_Memcheck(CeedQFunctionContext ctx) {
CeedQFunctionContext_Memcheck *impl;

// Free allocations and discard block ids
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedFree(&impl->data_allocated));
CeedCallBackend(CeedFree(&impl->data_owned));
if (impl->data_allocated) {
CeedCallBackend(CeedFree(&impl->data_allocated));
VALGRIND_DISCARD(impl->allocated_block_id);
}
if (impl->data_owned) {
CeedCallBackend(CeedFree(&impl->data_owned));
VALGRIND_DISCARD(impl->owned_block_id);
}
if (impl->data_borrowed) {
VALGRIND_DISCARD(impl->borrowed_block_id);
}
CeedCallBackend(CeedFree(&impl));
return CEED_ERROR_SUCCESS;
}
Expand Down
Loading