[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 21 13:14:08 PDT 2019


ABataev added inline comments.


================
Comment at: clang/lib/Sema/Sema.cpp:2108
 void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD,
                                    CapturedRegionKind K) {
+  CapturedRegionScopeInfo *CSI = new CapturedRegionScopeInfo(
----------------
jdenny wrote:
> ABataev wrote:
> > I would suggest adding a new parameter for CapturedLevel and the same parameter to `ActOnCapturedRegionStart` function. And pass the value for `CapturedLevel` explicitly at `ActOnCapturedRegionStart` function call in SemaOpenMP.cpp rather than rely on this solution.
> > Also, for each separate OpenMP directive, this counter must start from `0`. Currently, I think, it may lead to crash if we have several directives. Something like:
> > ```
> > #pragma omp parallel
> > #pragma omp target teams
> > ...
> > ```
> > I think it will work incorrectly for this example.
> I think the check that `OpenMPLevel` is the same prevents the problem you mention.
Ah, yes, missed this. Anyway, better to specify this explicitly as function parameters, I think, rather than trying to evaluate it here this way. We eventually may change the handling of captured regions in OpenMP, better to have everything in SemaOpenMP.cpp


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

https://reviews.llvm.org/D65835





More information about the cfe-commits mailing list