This patch is enabling the generation of clauses enum sets for semantics check in Flang through
tablegen. Enum sets and directive - sets map is generated by the new tablegen infrsatructure for OpenMP
and other directive languages.
The semantic checks for OpenMP are modified to use this newly generated map.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
flang/test/Semantics/omp-clause-validity01.f90 | ||
---|---|---|
461 | As a side note, This is supposed to be fine in Clang so I removed the check. I looked at the OpenMP 5.0 std and didn't see a restriction on reduction for task loop simd. |
flang/test/Semantics/omp-clause-validity01.f90 | ||
---|---|---|
461 | What's the current plan? Are we trying to cover OpenMP 5.0 Spec for semantics (it appears so)? | |
llvm/include/llvm/Frontend/OpenMP/OMP.td | ||
230 | Bear with me, what does 50 mean? | |
431–435 | For target enter and target exit, nowait is only allowed once. If it's allowed here, will this restriction be captured by the rules in target directive above? |
Thanks for the review. I just updates the patch and answered your questions.
flang/test/Semantics/omp-clause-validity01.f90 | ||
---|---|---|
461 | Clang just moved to 5.0as default and my guesses that we are targeting 5.0 as well since it is the current standard. | |
llvm/include/llvm/Frontend/OpenMP/OMP.td | ||
230 | The VersionedClause is defined as this: VersionedClause<VersionedClause<Clause c, int min = 1, int max = 0x7FFFFFFF> So here it means the clause is valid from version 5.0 and up. This is currently not used in Flang but in Clang it's used and I took the value from the old macros definition OMPKinds.def. So version 4.5 would 45 and so on. | |
431–435 | Good catch. I updated the patch and moved them in the correct set. |
flang/lib/Semantics/check-omp-structure.cpp | ||
---|---|---|
67 | I love these patches :) And we clean up Clang after! | |
flang/test/Semantics/omp-clause-validity01.f90 | ||
461 | If Flang has the equivalent to -fopenmp-version=XX or you just hardcode a version to be used, it should be possible to retain this error for the 4.5 case, right? | |
llvm/include/llvm/Frontend/OpenMP/OMP.td | ||
217 | Yay! :) | |
230 | FWIW, that is how we specify the version on the command line (for clang) as well. | |
llvm/utils/TableGen/DirectiveEmitter.cpp | ||
461 | Any reason this is flang specific? I guess we want to use the information in Clang too (soonish). |
This is great! Much neater than what I wrote for the combined constructs before and sharable with the rest of llvm :)
Thanks!
I love these patches :) And we clean up Clang after!