Skip to content

Feature/global indices#251

Open
reguly wants to merge 30 commits intomasterfrom
feature/global_indices
Open

Feature/global indices#251
reguly wants to merge 30 commits intomasterfrom
feature/global_indices

Conversation

@reguly
Copy link
Collaborator

@reguly reguly commented Mar 6, 2025

We can start reviewing/testing this - the generic kway code path seems to have started working. @bozbez please take a look.

@reguly reguly requested a review from bozbez March 6, 2025 20:04
@reguly
Copy link
Collaborator Author

reguly commented Mar 8, 2025

@bozbez I have the airfoil validation tests fail due to memory leaks. But I am reasonably sure those leaks were already there. Why are these caught now? (op_realloc seems to be leaking memory, no idea what to do about it...)

@bozbez
Copy link
Collaborator

bozbez commented Mar 8, 2025

ASAN/UBSAN weren't previously used - we might want some make flag to enable/disable these since I'm not sure we want them on by default (might cause linking difficulties with non ASAN/UBSAN code).

Looks like op_realloc allocates a new buffer if realloc doesn't maintain alignment but then doesn't clear up the old buffer.
I think we could just always op_malloc and copy in op_realloc since realloc might already be copying, or adding the free is easy enough anyway.

@reguly reguly force-pushed the feature/global_indices branch from 4a241fe to 68973a4 Compare April 16, 2025 20:26
@bozbez bozbez force-pushed the feature/global_indices branch from cf590f6 to ea2821b Compare November 11, 2025 12:05
@bozbez bozbez force-pushed the feature/global_indices branch from cab68f1 to 4acf135 Compare November 26, 2025 12:36
to; /* set pointed to */
int dim, /* dimension of pointer */
*map; /* array defining pointer */
idx_g_t *map_gbl; /* array with global indices (long type) */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it essential to add another pointer?
Cant we cast and get it when needed [the only use case "if (new_map->map_gbl[i] >= to_set_global_size)"]


op_map op_decl_map(op_set, op_set, int, int *, char const *);
op_map op_decl_map(op_set, op_set, int, idx_l_t *, char const *);
op_map op_decl_map_long(op_set, op_set, int, idx_g_t *, char const *);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it required to have two API calls, when we can overload the function?
For example,
Screenshot 2025-11-27 at 18 26 26

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't overload in pure C without that _Generic, and in this case I think just having it simpler (and not requiring us to do anything fancy in Fortran either) is probably for the best

@gihanmudalige
Copy link
Collaborator

Hi @bozbez and @reguly is this PR ready to be merged ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants