jhuber6 marked an inline comment as done.
jhuber6 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)
     {
----------------
jdoerfert wrote:
> 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?
The first struct is the one that I'm assuming comes from the OpenMP CodeGen 
that places the Ident_t struct in the IR file. if I declare a struct also named 
ident_t in the C source file it most likely will see that there's two structs 
with the same name and call the second one "ident_t.0" internally. The other 
ident_t struct is only known once clang generates the LLVM IR so I can't just 
use "ident_t" nor can I declare a struct with the same name.


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

Reply via email to