The following semantic check is removed in OpenMP Version 5.0:
Taskloop simd construct restrictions: No reduction clause can be specified.
Also fix several typos.
Differential D105874
[flang][OpenMP] Fix semantic check of test case in taskloop simd construct peixin on Jul 12 2021, 11:33 PM. Authored by
Details The following semantic check is removed in OpenMP Version 5.0: Taskloop simd construct restrictions: No reduction clause can be specified. Also fix several typos.
Diff Detail
Event TimelineComment Actions Thanks, @peixin for this patch. Also great to see you here. The recommendation is to provide the full context when submitting patches. I think if you use arcanist, this happens by default. If you use the web interface then there are some recommended commands to use to get the full context. Please see the following links. Also, could you point me to the place in the standard where this restriction is specified?
Comment Actions @kiranchandramohan Thanks for the guide to submit patches and the comments. The standard is at page 92 in openmp-4.5 specification, which I quote: Restrictions The restrictions for the taskloop and simd constructs apply. No reduction clause can be specified. I argee that saying the clause is not allowed in the directive is better. Changed the code fix and test case error info. It is common semantic check in DirectiveStructureChecker in flang/lib/Semantics/check-directive-structure.h, which I quote in the following: if (!GetContext().allowedClauses.test(clause) && !GetContext().allowedOnceClauses.test(clause) && !GetContext().allowedExclusiveClauses.test(clause) && !GetContext().requiredClauses.test(clause)) { context_.Say(GetContext().clauseSource, "%s clause is not allowed on the %s directive"_err_en_US, parser::ToUpperCaseLetters(getClauseName(clause).str()), parser::ToUpperCaseLetters(GetContext().directiveSource.ToString())); return; } The reason why it is not checked in DirectiveStructureChecker is the clause OMPC_Reduction is defined inside OMP_TaskLoopSimd in llvm/include/llvm/Frontend/OpenMP/OMP.td. Comment Actions
I think this is going against the generic directive checker. If the clause is set as allowed in the OMP.td file it should be allowed. Since the file is shared with clang the semantic should be done on version 5.0 or 5.1. Comment Actions @clementval Yes, I agree. I did not notice that this restriction was deleted in Openmp-5.0 standard when I upload the first version of this patch. Do you think if I should fix the restriction info in omp-taskloop-simd01.f90 and the typos in omp-clause-validity01.f90, and change the commit message by stating that this restriction is deleted in openmp version 5.0? Comment Actions Thanks for the comments. Update the following:
|
I think would be better to say that reduction is not expected for taskloop simd? Or is this a common pattern that we have in semantic errors?
I think we capitalize the name of constructs when we report errors. Please check other errors and capitalize if needed.