[PATCH] D110714: [Flang][openmp] Added semantic checks for atomic construct

[Nimish Mishra via Phabricator via llvm-commits]

NimishMishra created this revision.
NimishMishra added projects: Flang, OpenMP, LLVM.
Herald added subscribers: jdoerfert, guansong, yaxunl.
Herald added a reviewer: sscalpone.
NimishMishra requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, sstefan1.

The following patch implements the following semantic checks for atomic construct based on OpenMP 5.0 specifications.

- **In atomic update statement, binary operator is one of +, *, -, /, .AND., .OR., .EQV., or .NEQV.**

In //llvm-project/flang/lib/Semantics/check-omp-structure.cpp//, a new class //OmpAtomicConstructChecker// is added for all semantic checks implemented in this patch. This class is used in a //parser::Walk// in //OmpStructureChecker::CheckOmpAtomicConstructStructure// in //check-omp-structure.cpp// itself.

The entire aim is to, for any OpenMP atomic update statement, initiate a //parser::Walk// on the construct, capture the assignment statement in //bool Pre(const parser::AssignmentStmt &assignment)// inside //OmpAtomicConstructChecker//, and initiate a check with //CheckOperatorValidity// for the operator validity.

//CheckOperatorValidity// has two //HasMember// checks. The first check is to see whether the assignment statement has a binary operator or not. Because if it doesn't, the call to //CompareNames// is unsuccessful. The second check is to see if the operator is an allowed binary operator.

- **In atomic update statement, the statements must be of the form //x = x operator expr// or //x = expr operator x//**

The //CompareNames// function called from within //CheckOperatorValidity// checks that if we have an assignment statement with a binary operator, whether the variable names are same on the LHS and RHS.

- **In atomic update statement, only the following intrinsic procedures are allowed: MAX, MIN, IAND, IOR, or IEOR**

The functionality for the same is added in the function //CheckProcedureDesignatorValidity//.

-----------------------------

Alternative approaches tried which we could discuss upon:

- In //CheckOperatorValidity//, I earlier tried a //std::visit// approach. But that required us to enumerate all **dis**-allowed binary operators. Because had we emitted errors for anything that is not allowed, we began facing issues with assignment statements in atomic READ statements.

- In //llvm-project/flang/include/flang/Parser/parse-tree.h//, the //struct Expr// has a variant where all relevant operators and procedure indirection things are present. One interesting structure is //IntrinsicBinary// which is the base structure for all relevant binary operators in //parser::Expr//. However, we found no way to capture the different binary operators through this base structure. Concretely, I was thinking something like this:

std::visit(common::visitors{

  [&](const parser::Expr::IntrinsicBinary &x){
          // check for validity of variable names as well as operator validity   
      },
  [&](const auto &x){
          // got something other than a valid binary operator. Let it pass. 
      },

}, node);


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110714

Files:
  flang/lib/Semantics/check-omp-structure.cpp
  flang/lib/Semantics/check-omp-structure.h
  flang/test/Semantics/omp-atomic02.f90
  flang/test/Semantics/omp-atomic03.f90
  flang/test/Semantics/omp-atomic04.f90

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110714.375857.patch
Type: text/x-patch
Size: 18534 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210929/6d8a3bc4/attachment.bin>