[PATCH] D110502: [Flang][openmp] Add semantic checks for OpenMP critical construct name resolution

Thu Oct 7 08:55:47 PDT 2021

clementval added a comment.

In D110502#3048086 <https://reviews.llvm.org/D110502#3048086>, @NimishMishra wrote:

> In D110502#3034679 <https://reviews.llvm.org/D110502#3034679>, @kiranchandramohan wrote:
>
>> Thanks @NimishMishra for taking this forward. Can you include the parser changes also from https://reviews.llvm.org/D93051?
>
> Thanks for the comment. I analyzed this as well as worked on a few semantic checks related to hint clause. Are we sure that we require this as a clause list? Because everything is working smoothly anyway. In  D93051 <https://reviews.llvm.org/D93051>, parser changes involves changing the optional clause to a list, of which I do not presently understand the requirement.
>
> Also the clause list is no longer a //std::optional//. However, clauses are actually optional. But there exist examples in https://www.openmp.org/wp-content/uploads/openmp-examples-5.0.0.pdf on page 221 which indicates that it is indeed optional.

The clause list should never be optional. Only the clauses inside of the list should be optional or not. D93051 <https://reviews.llvm.org/D93051> is important to make sure we have a homogenous way of dealing with clauses and the OMP.td is correctly defined. Anyway D93051 <https://reviews.llvm.org/D93051> should probably be updated and landed separately if you don't bring these changes in.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110502