From 59ca403b295c706dbd15d4291b7fd34591403164 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Fri, 8 Aug 2025 14:46:40 -0500 Subject: [PATCH] [flang][OpenMP] Refactor creating atomic analysis, NFC Turn it into a class that combines the information and generates the analysis instead of having independent functions do it. --- flang/lib/Semantics/check-omp-atomic.cpp | 161 ++++++++++++++--------- 1 file changed, 97 insertions(+), 64 deletions(-) diff --git a/flang/lib/Semantics/check-omp-atomic.cpp b/flang/lib/Semantics/check-omp-atomic.cpp index fcb0f9ad1e25d..a5fe820b1069b 100644 --- a/flang/lib/Semantics/check-omp-atomic.cpp +++ b/flang/lib/Semantics/check-omp-atomic.cpp @@ -222,47 +222,77 @@ static void SetAssignment(parser::AssignmentStmt::TypedAssignment &assign, } } -static parser::OpenMPAtomicConstruct::Analysis::Op MakeAtomicAnalysisOp( - int what, - const std::optional &maybeAssign = std::nullopt) { - parser::OpenMPAtomicConstruct::Analysis::Op operation; - operation.what = what; - SetAssignment(operation.assign, maybeAssign); - return operation; -} +namespace { +struct AtomicAnalysis { + AtomicAnalysis(const SomeExpr &atom, const MaybeExpr &cond = std::nullopt) + : atom_(atom), cond_(cond) {} + + AtomicAnalysis &addOp0(int what, + const std::optional &maybeAssign = std::nullopt) { + return addOp(op0_, what, maybeAssign); + } + AtomicAnalysis &addOp1(int what, + const std::optional &maybeAssign = std::nullopt) { + return addOp(op1_, what, maybeAssign); + } -static parser::OpenMPAtomicConstruct::Analysis MakeAtomicAnalysis( - const SomeExpr &atom, const MaybeExpr &cond, - parser::OpenMPAtomicConstruct::Analysis::Op &&op0, - parser::OpenMPAtomicConstruct::Analysis::Op &&op1) { - // Defined in flang/include/flang/Parser/parse-tree.h - // - // struct Analysis { - // struct Kind { - // static constexpr int None = 0; - // static constexpr int Read = 1; - // static constexpr int Write = 2; - // static constexpr int Update = Read | Write; - // static constexpr int Action = 3; // Bits containing N, R, W, U - // static constexpr int IfTrue = 4; - // static constexpr int IfFalse = 8; - // static constexpr int Condition = 12; // Bits containing IfTrue, IfFalse - // }; - // struct Op { - // int what; - // TypedAssignment assign; - // }; - // TypedExpr atom, cond; - // Op op0, op1; - // }; - - parser::OpenMPAtomicConstruct::Analysis an; - SetExpr(an.atom, atom); - SetExpr(an.cond, cond); - an.op0 = std::move(op0); - an.op1 = std::move(op1); - return an; -} + operator parser::OpenMPAtomicConstruct::Analysis() const { + // Defined in flang/include/flang/Parser/parse-tree.h + // + // struct Analysis { + // struct Kind { + // static constexpr int None = 0; + // static constexpr int Read = 1; + // static constexpr int Write = 2; + // static constexpr int Update = Read | Write; + // static constexpr int Action = 3; // Bits containing None, Read, + // // Write, Update + // static constexpr int IfTrue = 4; + // static constexpr int IfFalse = 8; + // static constexpr int Condition = 12; // Bits containing IfTrue, + // // IfFalse + // }; + // struct Op { + // int what; + // TypedAssignment assign; + // }; + // TypedExpr atom, cond; + // Op op0, op1; + // }; + + parser::OpenMPAtomicConstruct::Analysis an; + SetExpr(an.atom, atom_); + SetExpr(an.cond, cond_); + an.op0 = std::move(op0_); + an.op1 = std::move(op1_); + return an; + } + +private: + struct Op { + operator parser::OpenMPAtomicConstruct::Analysis::Op() const { + parser::OpenMPAtomicConstruct::Analysis::Op op; + op.what = what; + SetAssignment(op.assign, assign); + return op; + } + + int what; + std::optional assign; + }; + + AtomicAnalysis &addOp(Op &op, int what, + const std::optional &maybeAssign) { + op.what = what; + op.assign = maybeAssign; + return *this; + } + + const SomeExpr &atom_; + const MaybeExpr &cond_; + Op op0_, op1_; +}; +} // namespace /// Check if `expr` satisfies the following conditions for x and v: /// @@ -805,9 +835,9 @@ void OmpStructureChecker::CheckAtomicUpdateOnly( CheckAtomicUpdateAssignment(*maybeUpdate, action.source); using Analysis = parser::OpenMPAtomicConstruct::Analysis; - x.analysis = MakeAtomicAnalysis(atom, std::nullopt, - MakeAtomicAnalysisOp(Analysis::Update, maybeUpdate), - MakeAtomicAnalysisOp(Analysis::None)); + x.analysis = AtomicAnalysis(atom) + .addOp0(Analysis::Update, maybeUpdate) + .addOp1(Analysis::None); } else if (!IsAssignment(action.stmt)) { context_.Say( source, "ATOMIC UPDATE operation should be an assignment"_err_en_US); @@ -889,9 +919,11 @@ void OmpStructureChecker::CheckAtomicConditionalUpdate( } using Analysis = parser::OpenMPAtomicConstruct::Analysis; - x.analysis = MakeAtomicAnalysis(assign.lhs, update.cond, - MakeAtomicAnalysisOp(Analysis::Update | Analysis::IfTrue, assign), - MakeAtomicAnalysisOp(Analysis::None)); + const SomeExpr &atom{assign.lhs}; + + x.analysis = AtomicAnalysis(atom, update.cond) + .addOp0(Analysis::Update | Analysis::IfTrue, assign) + .addOp1(Analysis::None); } void OmpStructureChecker::CheckAtomicUpdateCapture( @@ -936,13 +968,13 @@ void OmpStructureChecker::CheckAtomicUpdateCapture( } if (GetActionStmt(&body.front()).stmt == uact.stmt) { - x.analysis = MakeAtomicAnalysis(atom, std::nullopt, - MakeAtomicAnalysisOp(action, update), - MakeAtomicAnalysisOp(Analysis::Read, capture)); + x.analysis = AtomicAnalysis(atom) + .addOp0(action, update) + .addOp1(Analysis::Read, capture); } else { - x.analysis = MakeAtomicAnalysis(atom, std::nullopt, - MakeAtomicAnalysisOp(Analysis::Read, capture), - MakeAtomicAnalysisOp(action, update)); + x.analysis = AtomicAnalysis(atom) + .addOp0(Analysis::Read, capture) + .addOp1(action, update); } } @@ -1087,15 +1119,16 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture( evaluate::Assignment updAssign{*GetEvaluateAssignment(update.ift.stmt)}; evaluate::Assignment capAssign{*GetEvaluateAssignment(capture.stmt)}; + const SomeExpr &atom{updAssign.lhs}; if (captureFirst) { - x.analysis = MakeAtomicAnalysis(updAssign.lhs, update.cond, - MakeAtomicAnalysisOp(Analysis::Read | captureWhen, capAssign), - MakeAtomicAnalysisOp(Analysis::Write | updateWhen, updAssign)); + x.analysis = AtomicAnalysis(atom, update.cond) + .addOp0(Analysis::Read | captureWhen, capAssign) + .addOp1(Analysis::Write | updateWhen, updAssign); } else { - x.analysis = MakeAtomicAnalysis(updAssign.lhs, update.cond, - MakeAtomicAnalysisOp(Analysis::Write | updateWhen, updAssign), - MakeAtomicAnalysisOp(Analysis::Read | captureWhen, capAssign)); + x.analysis = AtomicAnalysis(atom, update.cond) + .addOp0(Analysis::Write | updateWhen, updAssign) + .addOp1(Analysis::Read | captureWhen, capAssign); } } @@ -1125,9 +1158,9 @@ void OmpStructureChecker::CheckAtomicRead( if (auto maybe{GetConvertInput(maybeRead->rhs)}) { const SomeExpr &atom{*maybe}; using Analysis = parser::OpenMPAtomicConstruct::Analysis; - x.analysis = MakeAtomicAnalysis(atom, std::nullopt, - MakeAtomicAnalysisOp(Analysis::Read, maybeRead), - MakeAtomicAnalysisOp(Analysis::None)); + x.analysis = AtomicAnalysis(atom) + .addOp0(Analysis::Read, maybeRead) + .addOp1(Analysis::None); } } else if (!IsAssignment(action.stmt)) { context_.Say( @@ -1159,9 +1192,9 @@ void OmpStructureChecker::CheckAtomicWrite( CheckAtomicWriteAssignment(*maybeWrite, action.source); using Analysis = parser::OpenMPAtomicConstruct::Analysis; - x.analysis = MakeAtomicAnalysis(atom, std::nullopt, - MakeAtomicAnalysisOp(Analysis::Write, maybeWrite), - MakeAtomicAnalysisOp(Analysis::None)); + x.analysis = AtomicAnalysis(atom) + .addOp0(Analysis::Write, maybeWrite) + .addOp1(Analysis::None); } else if (!IsAssignment(action.stmt)) { context_.Say( x.source, "ATOMIC WRITE operation should be an assignment"_err_en_US);