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.

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

Add kernel launcher array allocation alignment #1357

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

ZzEeKkAa
Copy link
Contributor

@ZzEeKkAa ZzEeKkAa commented Feb 23, 2024

Adds alignment for array allocations inside call_kernel. This resolves failing tests, that was triggering errors due to array overflows or something like this (first value of array type got overwritten to 0 in some test cases). Seems like alignment solved the problem, but further investigations are required.

Fixes #1308

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?

@ZzEeKkAa ZzEeKkAa self-assigned this Feb 23, 2024
@ZzEeKkAa ZzEeKkAa requested a review from diptorupd February 23, 2024 17:32
@ZzEeKkAa ZzEeKkAa force-pushed the fix/kernel_launcher_alignment branch from 660e71e to 211bd07 Compare February 23, 2024 17:38
@ZzEeKkAa ZzEeKkAa changed the title Add kernel launcher allocation alignment Add kernel launcher array allocation alignment Feb 23, 2024
@ZzEeKkAa ZzEeKkAa marked this pull request as ready for review February 23, 2024 18:11
@ZzEeKkAa ZzEeKkAa enabled auto-merge February 23, 2024 18:12
@ZzEeKkAa ZzEeKkAa disabled auto-merge February 23, 2024 18:12
@ZzEeKkAa ZzEeKkAa force-pushed the fix/kernel_launcher_alignment branch from 211bd07 to 492f4ad Compare February 23, 2024 18:12
Copy link
Contributor

@diptorupd diptorupd left a comment

Choose a reason for hiding this comment

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

Great job debugging this mysterious issue. We should look at adding address sanitation option in the CPUCodegen as an optional feature to help debugging these cases for future.

@ZzEeKkAa ZzEeKkAa enabled auto-merge February 23, 2024 18:17
@ZzEeKkAa
Copy link
Contributor Author

Great job debugging this mysterious issue. We should look at adding address sanitation option in the CPUCodegen as an optional feature to help debugging these cases for future.

Thank you for helping figuring out what's going on there! Sanitation will be really great feature. Github CIs` tests must be run with this flag on for more comprehensive testing.

@ZzEeKkAa ZzEeKkAa merged commit 6e28999 into main Feb 23, 2024
44 of 52 checks passed
@ZzEeKkAa ZzEeKkAa deleted the fix/kernel_launcher_alignment branch February 23, 2024 18:46
github-actions bot added a commit that referenced this pull request Feb 23, 2024
Add kernel launcher array allocation alignment 6e28999
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.

NdRange does not produce correct results depending on global size
2 participants