Skip to content

[Clang][OpenMP] Emit unsupported directive error #70233

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 3 commits into from
Nov 24, 2023

Conversation

rkchang
Copy link
Contributor

@rkchang rkchang commented Oct 25, 2023

Hello!

This PR fixes #63871. Clang should no longer crash and instead emits an error message.
@shiltian is this what you had in mind for an error message? I think I followed everything in [1] but I did not add a unit test as this behavior should change in the future when dispatch is implemented. Also I did not find any existing unit tests that test for behavior like this.

Below is an example of the new error message:

~/dev/fork-llvm-project omp_dispatch_unimpl
❯ ./install/bin/clang -fopenmp  -c -emit-llvm -Xclang -disable-llvm-passes test.c
test.c:6:5: error: cannot compile this OpenMP dispatch directive yet
    6 |     #pragma omp dispatch
      |     ^~~~~~~~~~~~~~~~~~~~
1 error generated.

Please let me know if there's anything you'd like me to change.

PS. This is my first open source contribution so if there's any etiquette or instructions I missed please let me know.

[1] https://llvm.org/docs/Contributing.html

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Oct 25, 2023
@shiltian shiltian changed the title Emit unsupported directive error [Clang][OpenMP] Emit unsupported directive error Oct 25, 2023
@shiltian
Copy link
Contributor

Can you add a small test to check the error message is correctly emitted? You can refer to those diagnosis tests under clang/test/OpenMP.

@rkchang rkchang force-pushed the omp_dispatch_unimpl branch from 837e288 to 65d6109 Compare November 22, 2023 06:09
@llvmbot llvmbot added the clang:openmp OpenMP related changes to Clang label Nov 22, 2023
@rkchang
Copy link
Contributor Author

rkchang commented Nov 22, 2023

Added a test case. Thanks for the pointer! Here's the result:

~/dev/fork-llvm-project omp_dispatch_unimpl
❯ llvm-lit -vv clang/test/OpenMP/dispatch_unsupported.c
llvm-lit: /home/rkchang/dev/fork-llvm-project/llvm/utils/lit/lit/llvm/config.py:488: note: using clang: /home/rkchang/dev/fork-llvm-project/build/bin/clang
-- Testing: 1 tests, 1 workers --
PASS: Clang :: OpenMP/dispatch_unsupported.c (1 of 1)

Testing Time: 0.07s

Total Discovered Tests: 1
  Passed: 1 (100.00%)

Copy link
Contributor

@shiltian shiltian left a comment

Choose a reason for hiding this comment

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

LG with a nit

@shiltian shiltian merged commit 85b2e9c into llvm:main Nov 24, 2023
@rkchang rkchang deleted the omp_dispatch_unimpl branch November 24, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang with assertion on crashed when openmp's 5.1 dispatch construct is used
3 participants