Beginning to add missing runtime functions to OMPKinds.def. I haven't added any of the target-offloading runtime functions yet. I've added the definitions to the attributes test file but have not added any attributes to OMPKinds.def.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thx! Can u make all of them at least nounwind? These are "only" ones clang currently generates, right?
I got most of them by getting the LLVM IR for the test files in openmp/runtime/test and took the unique calls, then filled in the gaps for some variants and others I knew of. There may be a few that Clang doesn't generate, I think kmpc_begin() is always implicit. I wasn't sure if I could declare them all as nounwind since there were some other kmpc* calls in the add_attributes.ll that didn't show any attributes so I was playing on the safe side. I could add them all as nounwind if it's safe.
Also I should mention that I just treated the task_t types as void pointers. I could find their definition in the LLVM IR but they were complicated and I felt like it wasn't worth the effort for now.
Good. We don't necessarily need internal ones but all that are generated help us.
I wasn't sure if I could declare them all as nounwind since there were some other kmpc* calls in the add_attributes.ll that didn't show any attributes so I was playing on the safe side. I could add them all as nounwind if it's safe.
nounwind is safe. Where it is missing we just haven't added it in the .def file.
Also I should mention that I just treated the task_t types as void pointers. I could find their definition in the LLVM IR but they were complicated and I felt like it wasn't worth the effort for now.
That should be fine for now.
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def | ||
---|---|---|
466 | This should be a void* and its corresponding definition an i8* but since the types come from the getXXXTy methods I wasn't sure how to get an i8***. |
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def | ||
---|---|---|
466 | No way yet, but I think this should work just fine: https://reviews.llvm.org/D79675#inline-730836 |
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def | ||
---|---|---|
431 | Functions like this one use "size_t" which would be a 32 bit integer on a 32 bit system, but I've just set it to 64 bits for now. Should I add some macro's to set it to an Int32 or Int64 depending on the target? The tests target a 64 bit system and I can't see many people using OpenMP on a 32 bit computer so I wasn't sure if I should bother. |
So should I just add his SizeTy wherever the function uses size_t? I could also do a define of VoidPtr and Int8Ptr to make it more obvious.
Adding missing runtimes and a void pointer type. Replaced uses of size_t with SizeTy instead of Int64, SizeTy isn't defined yet so it won't build.
OK, now we have cyclic dependences ;)
I would suggest you split the declarations with size_t type out and we commit this patch. Once @fghanim landed the other one we commit the size_t ones.
I just added a temporary type that sets it to Int64 so it will compile, then it can be removed once there's an external type. Does that work?
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def | ||
---|---|---|
290 | Forgot to change these to VoidPtr. |
@jdoerfert At this point should I just drop the changes I made in OMPKinds.def ? :D
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def | ||
---|---|---|
242 | @jdoerfert Should I drop these def. from my patch? | |
289 | @jdoerfert Also, these 3? | |
469 | @jdoerfert and these 2? | |
478 | @jdoerfert Also, these 2? |
The target dependent size stuff is not solved in here so your solution is needed. The runtime functions and types now declared do not need to be declared again by you.
I'll merge this today.
Ok. But this patch doesn't handle or initialize any of the voidptr types in OMPConstant.cpp/h which I need for my stuff. So wouldn't it be better, to just remove the things I already defined in my patch from here, so it would work for now, and then I'll come back and clean things up when everything is merged? what do you think?
You mean because of the Fortran/Flang void* uncertainty? I mean, this should initialize VoidPtr and VoidPtrPtr just fine, right?
@jdoerfert Should I drop these def. from my patch?