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

Possible bug in OpenMP implementation #502

Open
eZWALT opened this issue Mar 10, 2024 · 3 comments
Open

Possible bug in OpenMP implementation #502

eZWALT opened this issue Mar 10, 2024 · 3 comments

Comments

@eZWALT
Copy link
Contributor

eZWALT commented Mar 10, 2024

I am seeking insights regarding an issue encountered during testing. When including the <omp.h> library in the test code as shown below (Bearing in mind that this simplified #pragma omp parallel is already implemented):

#include <omp.h>

int main(){
     #pragma omp parallel{
     
     }
}

I observed unusual exceptions. This inclusion leads to the CIRGenerator.cpp file reaching a conditional statement that terminates with a llvm_unreachable. While I understand this may be expected behavior, I lack sufficient context to fully comprehend the issue. Could someone explain why the code functions without the inclusion of <omp.h> but raises an exception when included?

This is the most relevant part of the error, if it helps:

NYI
UNREACHABLE executed at /home/walterjtv/Escritorio/Q8/TFG/clangir/clang/lib/CIR/CodeGen/CIRGenerator.cpp:164!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /home/walterjtv/Escritorio/Q8/TFG/clangir-install/bin/clang-19 -fopenmp -fclangir-enable -emit-cir 2_basic_parallel.c -o 2_basic_parallel.cir
1.	/usr/include/x86_64-linux-gnu/bits/types.h:155:26: current parser token '__fsid_t'
2.	/usr/include/x86_64-linux-gnu/bits/types.h:155:12 <Spelling=/usr/include/x86_64-linux-gnu/bits/typesizes.h:73:24>: parsing struct/union body 'struct (unnamed at /usr/include/x86_64-linux-gnu/bits/types.h:155:12)'
3.	/usr/include/x86_64-linux-gnu/bits/types.h:155:12 <Spelling=/usr/include/x86_64-linux-gnu/bits/typesizes.h:73:24>: CIR generation of declaration 'struct (unnamed at /usr/include/x86_64-linux-gnu/bits/types.h:155:12)'

Thank you for your support and efforts!

@Lancern
Copy link
Member

Lancern commented Mar 11, 2024

The second line of the CIR error output gives the position of the error:

// For OpenMP emit declare reduction functions, if required.
if (astCtx->getLangOpts().OpenMP) {
llvm_unreachable("NYI");
}

Basically this code checks whether we have -fopenmp flag in the command line. If the flag is present, then we crash with llvm_unreachable. So if you are implementing OpenMP stuff, you will need to replace this llvm_unreachable with your implementation code.

Note that this code only gets executed when we meet a TagDecl (which represents a struct / class / enum / union) on the clang AST. The inclusing of omp.h introduces a bunch of TagDecls so you get the error once you include omp.h.

If you are not sure what to do at here, a good reference is always the original clang CodeGen module. The corresponding part in the clang CodeGen is:

if (Ctx->getLangOpts().OpenMP) {
for (Decl *Member : D->decls()) {
if (auto *DRD = dyn_cast<OMPDeclareReductionDecl>(Member)) {
if (Ctx->DeclMustBeEmitted(DRD))
Builder->EmitGlobal(DRD);
} else if (auto *DMD = dyn_cast<OMPDeclareMapperDecl>(Member)) {
if (Ctx->DeclMustBeEmitted(DMD))
Builder->EmitGlobal(DMD);
}
}
}

You can start by understanding what the original clang CodeGen is doing and port it to CIRGen accordingly.

@bcardosolopes
Copy link
Member

Thanks for the detailed explanation @Lancern.

@eZWALT another possibility is to mock the relevant constructs out of #include <omp.h> and use them directly into your testcase, so you don't need to handle all the things not yet supported from that header. It's also possible that under clang/test/OpenMP you will find those mocked versions to copy from.

@eZWALT
Copy link
Contributor Author

eZWALT commented Mar 14, 2024

Great explanation @Lancern , thanks for all the time invested, after implementing the task construct I will be checking this out!

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

No branches or pull requests

3 participants