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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits