[Mlir-commits] [flang] [llvm] [mlir] [flang][OpenMP] Enable tiling (PR #143715)

[Sergio Afonso]

================
@@ -418,6 +420,28 @@ static void processHostEvalClauses(lower::AbstractConverter &converter,
               beginClauseList =
                   &std::get<parser::OmpClauseList>(beginDirective.t);
 
+              // For now we check if there is an inner OpenMPLoopConstruct, and
+              // extract the size clause from there
+              const auto &nestedOptional =
+                  std::get<std::optional<parser::NestedConstruct>>(
+                      ompConstruct.t);
+              assert(nestedOptional.has_value() &&
+                     "Expected a DoConstruct or OpenMPLoopConstruct");
+              const auto *innerConstruct =
+                  std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
+                      &(nestedOptional.value()));
+              if (innerConstruct) {
+                const auto &innerLoopConstruct = innerConstruct->value();
+                const auto &innerBegin =
+                    std::get<parser::OmpBeginLoopDirective>(
+                        innerLoopConstruct.t);
+                const auto &innerDirective =
+                    std::get<parser::OmpLoopDirective>(innerBegin.t);
+                if (innerDirective.v == llvm::omp::Directive::OMPD_tile) {
+                  middleClauseList =
+                      &std::get<parser::OmpClauseList>(innerBegin.t);
+                }
+              }
----------------
skatrak wrote:

I think this should be done as part of `processEval` instead. We should be able to process tiling sizes in the host while following the current approach by replacing calls to `cp.processCollapse(...)` in the switch statement inside of the `processEval` lambda below with something like:
```c++
cp.processCollapse(loc, eval, hostInfo->ops, hostInfo->iv);
processSingleNestedIf([](Directive nestedDir) {
  return nestedDir == OMPD_tile;
});
```
The previous block would probably make sense to refactor it into its own lambda (later on, we can actually turn a bunch of these lambdas into static functions, since that function has already become so complex), to avoid duplication since it's probably going to need modifications as more loop transformations are added.

Then, a case for that directive should be added to the same switch statement:
```c++
case OMPD_tile:
  cp.processSizes(...);
  break;
```

And I'm thinking we would also have to update the `HostEvaluatedOperands` structure, add `HostEvalInfo::apply` method(s) and update `collectValues/bindOperands`, so that we can properly forward these new host-evaluated clauses and prevent them from being re-evaluated inside of the target region.

https://github.com/llvm/llvm-project/pull/143715