Skip to content

[OpenMP] Use simple VLA implementation to replace uses of actual VLA #71412

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

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

shiltian
Copy link
Contributor

@shiltian shiltian commented Nov 6, 2023

Use of VLA can cause compile warning that was introduced in D156565. This patch
implements a simple stack/heap-based VLA that can miminc the behavior of an
actual VLA and prevent the warning. By default the stack accomodates the
elements. If the number of emelements is greater than N, which by default is 8,
a heap buffer will be allocated and used to acccomodate the elements.

@shiltian shiltian added the openmp:libomp OpenMP host runtime label Nov 6, 2023
Copy link
Contributor

@unterumarmung unterumarmung left a comment

Choose a reason for hiding this comment

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

I'm not an OpenMP contributor/maintainer/developer, so feel free to dismiss my suggestions.

@shiltian
Copy link
Contributor Author

shiltian commented Nov 6, 2023

I'm not an OpenMP contributor/maintainer/developer, so feel free to dismiss my suggestions.

Thanks for the comments. They are very helpful!

Copy link
Member

@jdoerfert jdoerfert left a comment

Choose a reason for hiding this comment

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

Can we have a very simple, non-growable, SmallVector-like API?
So provide a compile time bound for the size that could live on the stack, and if it's too large at runtime do a alloc/free.
The cost for this is basically nothing, but you avoid potential allocation cost for 95% of all cases (size <= 16 or 32).

Use of VLA can cause compile warning that was introduced in D156565. This patch
implements a simple stack/heap-based VLA that can miminc the behavior of an
actual VLA and prevent the warning. By default the stack accomodates the
elements. If the number of emelements is greater than N, which by default is 8,
a heap buffer will be allocated and used to acccomodate the elements.
@shiltian
Copy link
Contributor Author

Gentle ping

Copy link
Member

@jdoerfert jdoerfert left a comment

Choose a reason for hiding this comment

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

LG, thx

@shiltian shiltian merged commit eaab947 into llvm:main Nov 28, 2023
@shiltian shiltian deleted the vla-warning branch November 28, 2023 23:30
shiltian added a commit that referenced this pull request Nov 28, 2023
…ual VLA (#71412)"

This reverts commit eaab947 because it
causes link error.
@shiltian shiltian restored the vla-warning branch November 28, 2023 23:41
shiltian added a commit that referenced this pull request Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomp OpenMP host runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants