jdoerfert added inline comments.
================ Comment at: openmp/runtime/test/tasking/kmp_taskloop.c:100 th_counter[i] = 0; - #pragma omp parallel num_threads(N) + #pragma omp parallel // num_threads(N) { ---------------- jhuber6 wrote: > AndreyChurbanov wrote: > > jhuber6 wrote: > > > jhuber6 wrote: > > > > jdoerfert wrote: > > > > > jhuber6 wrote: > > > > > > I am not entirely sure why, but commenting this out causes the > > > > > > problem to go away. I tried adding proper names to the > > > > > > forward-declared functions but since clang already knew I had > > > > > > something called ident_t, I couldn't declare a new struct with the > > > > > > same name. > > > > > This is not good. The difference should only be that the > > > > > `kmpc_fork_call` has a different argument, right? Does the segfault > > > > > happen at compile or runtime? > > > > > > > > > > You can just use the ident_t clang created, right? Did you print the > > > > > function names requested by clang as we discussed? > > > > I added an assertion and debug statements. If I try to declare a struct > > > > named "Ident_t" I get the following error message in the seg-fault. I > > > > think the seg-fault is compile-time. > > > > > > > > Found OpenMP runtime function __kmpc_global_thread_num with type i32 > > > > (%struct.ident_t.0*). Expected type is i32 (%struct.ident_t*) > > > > clang: > > > > /home/jhuber/Documents/llvm-project/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:124: > > > > static llvm::Function* > > > > llvm::OpenMPIRBuilder::getOrCreateRuntimeFunction(llvm::Module&, > > > > llvm::omp::RuntimeFunction): Assertion `FnTy == Fn->getFunctionType() > > > > && "Found OpenMP runtime function has mismatched types"' failed. > > > I'm not sure if there's a way around this without changing the > > > getOrCreateRuntimeFunction method to return a FunctionCallee and removing > > > the assertion. Clang doesn't know about the ident_t struct when it's > > > compiling the file, but when its doing the codegen it sees two structs > > > with the same name and creates a new name. So when it gets the types it > > > says that ident_t and ident_t.0 don't match. As you said the old version > > > got around this by adding a bitcasting instruction so it knew how to turn > > > it into an ident_t pointer. > > Note that this change breaks the test on any system with more that 4 procs. > > Because array th_counter[4] is indexed by thread number which can easily > > be greater than 3 if number of threads is not limited. > The problem was that the num_threads clause required an implicit call to > kmpc_global_thread_num so it could be passed to kmpc_push_num_threads. The > types of the implicit function and the forward declaration then wouldn't > match up. I added another forward declaration to explicitly call > kmpc_push_num_threads. Is this a sufficient solution? We need this to work with num_threads(8). > Clang doesn't know about the ident_t struct when it's compiling the file, but > when its doing the codegen it sees two structs with the same name and creates > a new name. Where are the two structs coming from? We should have one. If clang introduces one it needs to use the one from OMPKindes.def instead. Is that a fix? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80222/new/ https://reviews.llvm.org/D80222 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits