[PATCH] D81478: [OPENMP50]Codegen for scan directives in parallel for regions.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3627
+      emitScanBasedDirective(CGF, S, NumIteratorsGen, FirstGen, SecondGen);
+    } else {
+      OMPCancelStackRAII CancelRegion(CGF, OMPD_parallel_for, S.hasCancel());
----------------
ABataev wrote:
> jdoerfert wrote:
> > This looks pretty much like D81658, right? Could we avoid the duplication and maybe use a templated function (or something else)?
> The duplication is quite small. Here we don't need to check for lastprivates update, we need to check for the cancellation region. I don't think that the outlined functions are going to be much simpler and easier to read.
I mean, these ~25 lines will exist 3 times at least. 2 times tacking lastprivate, which we can do for the third time at no extra cost, and 2 times with the cancelation RAII. Any change will need to be duplicated 3 times as well, etc.

If we do
```
template<typename RAII>
void emitWorksharingHelper(..., bool &HasLastprivate)
    if (llvm::any_of(S.getClausesOfKind<OMPReductionClause>(),
                     [](const OMPReductionClause *C) {
                       return C->getModifier() == OMPC_REDUCTION_inscan;
                     })) {
      const auto &&NumIteratorsGen = [&S](CodeGenFunction &CGF) {
        OMPLocalDeclMapRAII Scope(CGF);
        OMPLoopScope LoopScope(CGF, S);
        return CGF.EmitScalarExpr(S.getNumIterations());
      };
      const auto &&FirstGen = [&S](CodeGenFunction &CGF) {
        RAII CancelRegion(CGF, OMPD_for, S.hasCancel());
        (void)CGF.EmitOMPWorksharingLoop(S, S.getEnsureUpperBound(),
                                         emitForLoopBounds,
                                         emitDispatchForLoopBounds);
        // Emit an implicit barrier at the end.
        CGF.CGM.getOpenMPRuntime().emitBarrierCall(CGF, S.getBeginLoc(),
                                                   OMPD_for);
      };
      const auto &&SecondGen = [&S, &HasLastprivate](CodeGenFunction &CGF) {
        RAII CancelRegion(CGF, OMPD_for, S.hasCancel());
        HasLastprivate = CGF.EmitOMPWorksharingLoop(S, S.getEnsureUpperBound(),
                                         emitForLoopBounds,
                                         emitDispatchForLoopBounds);
      };
      emitScanBasedDirective(CGF, S, NumIteratorsGen, FirstGen, SecondGen);
    } else {
      OMPCancelStackRAII CancelRegion(CGF, OMPD_parallel_for, S.hasCancel());
        CGF.EmitOMPWorksharingLoop(S, S.getEnsureUpperBound(), emitForLoopBounds,
                                   emitDispatchForLoopBounds);
    }
```
We can call it three times:
```
emitWorksharingHelper<OMPCancelStackRAII>(...);
emitWorksharingHelper<OMPCancelStackRAII>(...);
emitWorksharingHelper<OMPDummyRAII>(...);
```

Wouldn't that be cleaner?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81478