fghanim added a comment.

@jdoerfert To answer your question as to why it is being codegened as shared, 
codegen doesn't handle `default` clause- at least I didn't come across such a 
thing- and if you think about it, it shouldn't need to. You either have `none` 
or `shared`:
- `none` is largly to throw diag errors - so clang either never leaves sema 
because it threw an error, or there are no errors, which suggests that the 
programmer has already listed all captured variables as other clauses. - either 
way, codegen doesn't need to handle it.
- `shared` is the default anyway, and codegen already makes any captured 
variableby the outlined function not "claimed" by any other privatization 
clause, shared - it just passes it as an arg. to the outlined function.

and Since @atmnpatel didn't add any new codegen for the `default{firstprivate}` 
case, codegen defaulted to shared - at least that's what I think happened.

@atmnpatel  The way privatization works currently in codegen, you go over every 
clause, find the list of variables for that clause, find some relevant info 
about the variable along with the proper way to initialize the variable (i.e. 
what constructor to use), and add it to list of all variables to be privatized, 
once done with all privatization clauses, you do codegen for privatization for 
all of them in one go. The exception to this is `copyin`. 
So what you may want to do is to handle the `default` clause in codegen. if 
it's `none` or `shared`, you ignore it. if it is `firstprivate` or `private`, 
find the captured variables of the soon to be outlined function that have not 
been captured by any other privatization clause, then figure a way to pass it 
to `codegenfunction::emitOMPFirstprivateClause` or  
`codegenfunction::emitPrivateClause`. Alternatively, find this list while 
building the AST, and add them as default captured variables, which will make 
codegen much easier. You are already checking for them to throw the `none` diag 
error anyway.



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7087
 
+/// Matches if the OpenMP ``default`` clause has ``shared`` kind specified.
+///
----------------
fix comment: ... has ``firstprivate`` kind ...


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4993
 
   // Check variables in the clauses if default(none) was specified.
+  if (DSAStack->getDefaultDSA() == DSA_none ||
----------------
fix the comments to say firstprivate as well (here and elsewhere)


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:5102
       Diag(P.second->getExprLoc(), diag::err_omp_no_dsa_for_variable)
           << P.first << P.second->getSourceRange();
       Diag(DSAStack->getDefaultDSALocation(), diag::note_omp_default_dsa_none);
----------------
Why is `firstprivate` throwing this error? isn't the purpose of specifying it 
as `default` is if a variable is not specified as anything, then it is 
automatically handled as `firstprivate`? or am I misunderstanding something?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75591/new/

https://reviews.llvm.org/D75591



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to