ABataev added a comment. In D65835#1639739 <https://reviews.llvm.org/D65835#1639739>, @jdenny wrote:
> In D65835#1639700 <https://reviews.llvm.org/D65835#1639700>, @ABataev wrote: > > > Chwck 2 ifs: `if (IsVariableUsedInMapClause)` and the second `if (IsByRef > > && Ty.getNonReferenceType()->isScalarType())`. If the variable is mapped, > > `IsByRef` is set to `true`, but later it may be changed in the second if, > > when we check for the use in the `firstprivate` clause. We need to add a > > check, that the `IsByRef` must be set to `false` onlly if > > `IsVariableUsedInMapClause` is set to `false` too for combined target > > constructs. > > > I've tried that. It results in capturing by reference on both the target > region and the teams region in the case of a combined target teams construct. > Thus, we end up with different codegen than when the directives are > separate, where capturing by reference is set only on the target region. > > The changes I just backed out handle this correctly because they distinguish > between the target and teams regions. Ahh, I see, in this case, the codegen for the inner subregion is changed. Ok, I see now. Then yes, we need this change with subregion counter. In this case, I would recommend trying to fix https://bugs.llvm.org/show_bug.cgi?id=40253 at first, as it relates to the same problem, I think. Better to fix the problem separately and then extend it for new feature rather than add 2 new features in one patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
