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 or x = intrinsic_procedure_name (expr_list, x) or x = intrinsic_procedure_name (x, expr_list)
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.
- At most one memory-order-clause may appear on the construct.
The overloaded function void OmpStructureChecker::CheckAtomicMemoryOrderClause checks for this through a simple counting of memory order clauses.
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);
Nit: Other checks do not mention Omp.