[PATCH] D110270: [flang][OpenMP] Check for occurrence of multiple list items in nontemporal clause for simd directive

[Arnamoy B via Phabricator via llvm-commits]

arnamoy10 added inline comments.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:184-192
+    for (auto const &var : alignedNameList) {
+      if (listVars.count(*(var.symbol)) == 1) {
+        context_.Say(itr->second->source,
+            "List item '%s' present at multiple ALIGNED clauses"_err_en_US,
+            var.ToString());
+        break;
+      }
----------------
kiranchandramohan wrote:
> arnamoy10 wrote:
> > kiranchandramohan wrote:
> > > Can you make this a lambda function that takes in a list of variables, with listvars declared inside the lambda and an argument (pass ALIGNED or NONTEMPORAL to it)?
> > Thanks for the comment.  I was creating a lambda as per your suggestion and passing `alignedClauses`/ `nonTemporalClauses` as the argument and another argument to denote whether we are processing ALIGNED or NONTEMPORAL.
> > 
> > However, in that lambda, how do I declare a generic variable, which will be defined as the following pseudocode based on the passed argument?  It cannot be `auto` e.g. the following will not work.
> > 
> > ```
> > auto &Clause;
> > if (arg == ALIGNED) 
> >   Clause = std::get<parser::OmpClause::Aligned>(itr->second->u)};
> > else
> >   Clause = std::get<parser::OmpClause::Aligned>(itr->second->u)};
> > ```
> > What will be the type of `Clause` in this case?
> I think the lambda at that level might not work since templates are not properly supported for lambdas in C++17. I was thinking something like the following (templated functions) will work. It can be called as `checkMultiple<parser::OmpClause::Aligned>(*itr, "ALIGNED")`. But I see that there is some difference in the way the namelist is constructed. So there are  additional checks or conditions required.
> ```
> const auto &alignedNameList{std::get<std::list<parser::Name>>(alignedClause.v.t)};
> const auto &nontempNameList{nontempClause.v};
> ```
> 
> ```
>   template<typename C, typename V>
> void checkMultiple(const V &v, const std::string &clauseName) {
> 
>     semantics::UnorderedSymbolSet listVars;
>   auto checkMultipleOcurrence = [&](const std::list<parser::Name>& nameList, const parser::CharBlock& item) {
>     for (auto const &var : nameList) {
>       if (listVars.count(*(var.symbol)) == 1) {
>         context_.Say(item,
>             "List item '%s' present at multiple %s clauses"_err_en_US,
>             var.ToString(), clauseName);
>         break;
>       }
>       listVars.insert(*(var.symbol));
>     }
>   };
>     const auto &alignedClause{
>         std::get<C>(v.second->u)};
>     const auto &alignedNameList{
>         std::get<std::list<parser::Name>>(alignedClause.v.t)};
>     checkMultipleOcurrence(alignedNameList, v.second->source);
> 
> }
> ```
> 
> Instead, you can try the lambda `checkMultipleOcurrence` that is there in the function above. I think this is what I suggested.
Thank you.  However, we could not fit the `alignedClause` and `alignedNameList' inside the helper because at compile time, the namelist construction could not be controlled.  Still used a lambda, please check the updated patch.


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

https://reviews.llvm.org/D110270