https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/98387
>From bc7e6962de9e1e44773eee89ccccd0f552342b40 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <krzysztof.parzys...@amd.com> Date: Wed, 10 Jul 2024 14:38:57 -0500 Subject: [PATCH 1/3] [clang][OpenMP] Fix region nesting check for `scan` directive The previous check was inconsistent. For example, it would allow ``` #pragma omp target #pragma omp parallel for for (...) { #pragma omp scan } ``` but not ``` #pragma omp target parallel for for (...) { #pragma omp scan } ``` Make the check conform to the wording on the specification. --- clang/lib/Sema/SemaOpenMP.cpp | 21 ++++++++++++------- .../test/OpenMP/Inputs/nesting_of_regions.cpp | 14 ++----------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index bc5c172f1390e..ef09e53077f47 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -4989,14 +4989,19 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, OrphanSeen = ParentRegion == OMPD_unknown; Recommend = ShouldBeInTargetRegion; } else if (CurrentRegion == OMPD_scan) { - // OpenMP [2.16, Nesting of Regions] - // If specified, a teams construct must be contained within a target - // construct. - NestingProhibited = - SemaRef.LangOpts.OpenMP < 50 || - (ParentRegion != OMPD_simd && ParentRegion != OMPD_for && - ParentRegion != OMPD_for_simd && ParentRegion != OMPD_parallel_for && - ParentRegion != OMPD_parallel_for_simd); + if (SemaRef.LangOpts.OpenMP >= 50) { + SmallVector<OpenMPDirectiveKind, 4> LeafOrComposite; + std::ignore = getLeafOrCompositeConstructs(ParentRegion, LeafOrComposite); + // OpenMP spec 5.0 and 5.1 require scan to be directly enclosed by for, + // simd, or for simd. This has to take into account combined directives. + // In 5.2 this seems to be implied by the fact that the specified + // separated constructs are do, for, and simd. + OpenMPDirectiveKind Enclosing = LeafOrComposite.back(); + NestingProhibited = Enclosing != OMPD_for && Enclosing != OMPD_simd && + Enclosing != OMPD_for_simd; + } else { + NestingProhibited = true; + } OrphanSeen = ParentRegion == OMPD_unknown; Recommend = ShouldBeInLoopSimdRegion; } diff --git a/clang/test/OpenMP/Inputs/nesting_of_regions.cpp b/clang/test/OpenMP/Inputs/nesting_of_regions.cpp index 969ddfcce4cb0..985cdc0e19adc 100644 --- a/clang/test/OpenMP/Inputs/nesting_of_regions.cpp +++ b/clang/test/OpenMP/Inputs/nesting_of_regions.cpp @@ -5346,11 +5346,6 @@ void foo() { } #pragma omp target parallel for for (int i = 0; i < 10; ++i) { -#pragma omp scan // expected-error {{region cannot be closely nested inside 'target parallel for' region}} - bar(); - } -#pragma omp target parallel for - for (int i = 0; i < 10; ++i) { #pragma omp taskwait bar(); } @@ -7146,7 +7141,7 @@ void foo() { } #pragma omp target simd for (int i = 0; i < 10; ++i) { -#pragma omp scan // omp45-error {{OpenMP constructs may not be nested inside a simd region}} omp50-error {{region cannot be closely nested inside 'target simd' region; perhaps you forget to enclose 'omp scan' directive into a for, simd, for simd, parallel for, or parallel for simd region?}} omp51-error {{region cannot be closely nested inside 'target simd' region; perhaps you forget to enclose 'omp scan' directive into a for, simd, for simd, parallel for, or parallel for simd region?}} +#pragma omp scan // omp45-error {{OpenMP constructs may not be nested inside a simd region}} omp50-error {{exactly one of 'inclusive' or 'exclusive' clauses is expected}} omp51-error {{exactly one of 'inclusive' or 'exclusive' clauses is expected}} bar(); } #pragma omp target simd @@ -14583,11 +14578,6 @@ void foo() { } #pragma omp target parallel for for (int i = 0; i < 10; ++i) { -#pragma omp scan // expected-error {{region cannot be closely nested inside 'target parallel for' region}} - bar(); - } -#pragma omp target parallel for - for (int i = 0; i < 10; ++i) { #pragma omp taskwait bar(); } @@ -16685,7 +16675,7 @@ void foo() { } #pragma omp target simd for (int i = 0; i < 10; ++i) { -#pragma omp scan // omp45-error {{OpenMP constructs may not be nested inside a simd region}} omp50-error {{region cannot be closely nested inside 'target simd' region; perhaps you forget to enclose 'omp scan' directive into a for, simd, for simd, parallel for, or parallel for simd region?}} omp51-error {{region cannot be closely nested inside 'target simd' region; perhaps you forget to enclose 'omp scan' directive into a for, simd, for simd, parallel for, or parallel for simd region?}} +#pragma omp scan // omp45-error {{OpenMP constructs may not be nested inside a simd region}} omp50-error {{exactly one of 'inclusive' or 'exclusive' clauses is expected}} omp51-error {{exactly one of 'inclusive' or 'exclusive' clauses is expected}} bar(); } #pragma omp target simd >From 263c2357cd686e63b5b78985d3c8c6f3635e69cc Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <krzysztof.parzys...@amd.com> Date: Wed, 10 Jul 2024 09:35:56 -0500 Subject: [PATCH 2/3] [clang][OpenMP] Remove compound directives from `checkNestingOfRegions` Express the constraints via constituent directives. --- clang/lib/Sema/SemaOpenMP.cpp | 107 +++++++++++++++++----------------- 1 file changed, 53 insertions(+), 54 deletions(-) diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index ef09e53077f47..5861923582eb1 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -4797,6 +4797,12 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, ShouldBeInTeamsRegion, ShouldBeInLoopSimdRegion, } Recommend = NoRecommend; + + SmallVector<OpenMPDirectiveKind, 4> LeafOrComposite; + ArrayRef<OpenMPDirectiveKind> ParentLOC = + getLeafOrCompositeConstructs(ParentRegion, LeafOrComposite); + OpenMPDirectiveKind EnclosingConstruct = ParentLOC.back(); + if (SemaRef.LangOpts.OpenMP >= 51 && Stack->isParentOrderConcurrent() && CurrentRegion != OMPD_simd && CurrentRegion != OMPD_loop && CurrentRegion != OMPD_parallel && @@ -4805,6 +4811,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, << getOpenMPDirectiveName(CurrentRegion); return true; } + if (isOpenMPSimdDirective(ParentRegion) && ((SemaRef.LangOpts.OpenMP <= 45 && CurrentRegion != OMPD_ordered) || (SemaRef.LangOpts.OpenMP >= 50 && CurrentRegion != OMPD_ordered && @@ -4828,19 +4835,20 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, << (SemaRef.LangOpts.OpenMP >= 50 ? 1 : 0); return CurrentRegion != OMPD_simd; } - if (ParentRegion == OMPD_atomic) { + + if (EnclosingConstruct == OMPD_atomic) { // OpenMP [2.16, Nesting of Regions] // OpenMP constructs may not be nested inside an atomic region. SemaRef.Diag(StartLoc, diag::err_omp_prohibited_region_atomic); return true; } + if (CurrentRegion == OMPD_section) { // OpenMP [2.7.2, sections Construct, Restrictions] // Orphaned section directives are prohibited. That is, the section // directives must appear within the sections construct and must not be // encountered elsewhere in the sections region. - if (ParentRegion != OMPD_sections && - ParentRegion != OMPD_parallel_sections) { + if (EnclosingConstruct != OMPD_sections) { SemaRef.Diag(StartLoc, diag::err_omp_orphaned_section_directive) << (ParentRegion != OMPD_unknown) << getOpenMPDirectiveName(ParentRegion); @@ -4848,6 +4856,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, } return false; } + // Allow some constructs (except teams and cancellation constructs) to be // orphaned (they could be used in functions, called from OpenMP regions // with the required preconditions). @@ -4856,18 +4865,20 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, CurrentRegion != OMPD_cancellation_point && CurrentRegion != OMPD_cancel && CurrentRegion != OMPD_scan) return false; + // Checks needed for mapping "loop" construct. Please check mapLoopConstruct // for a detailed explanation if (SemaRef.LangOpts.OpenMP >= 50 && CurrentRegion == OMPD_loop && (BindKind == OMPC_BIND_parallel || BindKind == OMPC_BIND_teams) && (isOpenMPWorksharingDirective(ParentRegion) || - ParentRegion == OMPD_loop)) { + EnclosingConstruct == OMPD_loop)) { int ErrorMsgNumber = (BindKind == OMPC_BIND_parallel) ? 1 : 4; SemaRef.Diag(StartLoc, diag::err_omp_prohibited_region) << true << getOpenMPDirectiveName(ParentRegion) << ErrorMsgNumber << getOpenMPDirectiveName(CurrentRegion); return true; } + if (CurrentRegion == OMPD_cancellation_point || CurrentRegion == OMPD_cancel) { // OpenMP [2.16, Nesting of Regions] @@ -4881,27 +4892,17 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, // construct-type-clause is not taskgroup must be closely nested inside an // OpenMP construct that matches the type specified in // construct-type-clause. - NestingProhibited = - !((CancelRegion == OMPD_parallel && - (ParentRegion == OMPD_parallel || - ParentRegion == OMPD_target_parallel)) || - (CancelRegion == OMPD_for && - (ParentRegion == OMPD_for || ParentRegion == OMPD_parallel_for || - ParentRegion == OMPD_target_parallel_for || - ParentRegion == OMPD_distribute_parallel_for || - ParentRegion == OMPD_teams_distribute_parallel_for || - ParentRegion == OMPD_target_teams_distribute_parallel_for)) || - (CancelRegion == OMPD_taskgroup && - (ParentRegion == OMPD_task || - (SemaRef.getLangOpts().OpenMP >= 50 && - (ParentRegion == OMPD_taskloop || - ParentRegion == OMPD_master_taskloop || - ParentRegion == OMPD_masked_taskloop || - ParentRegion == OMPD_parallel_masked_taskloop || - ParentRegion == OMPD_parallel_master_taskloop)))) || - (CancelRegion == OMPD_sections && - (ParentRegion == OMPD_section || ParentRegion == OMPD_sections || - ParentRegion == OMPD_parallel_sections))); + ArrayRef<OpenMPDirectiveKind> Leafs = getLeafConstructsOrSelf(ParentRegion); + if (CancelRegion == OMPD_taskgroup) { + NestingProhibited = EnclosingConstruct != OMPD_task && + (SemaRef.getLangOpts().OpenMP < 50 || + EnclosingConstruct != OMPD_taskloop); + } else if (CancelRegion == OMPD_sections) { + NestingProhibited = EnclosingConstruct != OMPD_section && + EnclosingConstruct != OMPD_sections; + } else { + NestingProhibited = CancelRegion != Leafs.back(); + } OrphanSeen = ParentRegion == OMPD_unknown; } else if (CurrentRegion == OMPD_master || CurrentRegion == OMPD_masked) { // OpenMP 5.1 [2.22, Nesting of Regions] @@ -4942,13 +4943,12 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, // OpenMP 5.1 [2.22, Nesting of Regions] // A barrier region may not be closely nested inside a worksharing, loop, // task, taskloop, critical, ordered, atomic, or masked region. - NestingProhibited = - isOpenMPWorksharingDirective(ParentRegion) || - isOpenMPGenericLoopDirective(ParentRegion) || - isOpenMPTaskingDirective(ParentRegion) || ParentRegion == OMPD_master || - ParentRegion == OMPD_masked || ParentRegion == OMPD_parallel_master || - ParentRegion == OMPD_parallel_masked || ParentRegion == OMPD_critical || - ParentRegion == OMPD_ordered; + NestingProhibited = isOpenMPWorksharingDirective(ParentRegion) || + isOpenMPGenericLoopDirective(ParentRegion) || + isOpenMPTaskingDirective(ParentRegion) || + llvm::is_contained({OMPD_masked, OMPD_master, + OMPD_critical, OMPD_ordered}, + EnclosingConstruct); } else if (isOpenMPWorksharingDirective(CurrentRegion) && !isOpenMPParallelDirective(CurrentRegion) && !isOpenMPTeamsDirective(CurrentRegion)) { @@ -4956,13 +4956,12 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, // A loop region that binds to a parallel region or a worksharing region // may not be closely nested inside a worksharing, loop, task, taskloop, // critical, ordered, atomic, or masked region. - NestingProhibited = - isOpenMPWorksharingDirective(ParentRegion) || - isOpenMPGenericLoopDirective(ParentRegion) || - isOpenMPTaskingDirective(ParentRegion) || ParentRegion == OMPD_master || - ParentRegion == OMPD_masked || ParentRegion == OMPD_parallel_master || - ParentRegion == OMPD_parallel_masked || ParentRegion == OMPD_critical || - ParentRegion == OMPD_ordered; + NestingProhibited = isOpenMPWorksharingDirective(ParentRegion) || + isOpenMPGenericLoopDirective(ParentRegion) || + isOpenMPTaskingDirective(ParentRegion) || + llvm::is_contained({OMPD_masked, OMPD_master, + OMPD_critical, OMPD_ordered}, + EnclosingConstruct); Recommend = ShouldBeInParallelRegion; } else if (CurrentRegion == OMPD_ordered) { // OpenMP [2.16, Nesting of Regions] @@ -4973,7 +4972,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, // OpenMP [2.8.1,simd Construct, Restrictions] // An ordered construct with the simd clause is the only OpenMP construct // that can appear in the simd region. - NestingProhibited = ParentRegion == OMPD_critical || + NestingProhibited = EnclosingConstruct == OMPD_critical || isOpenMPTaskingDirective(ParentRegion) || !(isOpenMPSimdDirective(ParentRegion) || Stack->isParentOrderedRegion()); @@ -4983,31 +4982,29 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, // If specified, a teams construct must be contained within a target // construct. NestingProhibited = - (SemaRef.LangOpts.OpenMP <= 45 && ParentRegion != OMPD_target) || - (SemaRef.LangOpts.OpenMP >= 50 && ParentRegion != OMPD_unknown && - ParentRegion != OMPD_target); + (SemaRef.LangOpts.OpenMP <= 45 && EnclosingConstruct != OMPD_target) || + (SemaRef.LangOpts.OpenMP >= 50 && EnclosingConstruct != OMPD_unknown && + EnclosingConstruct != OMPD_target); OrphanSeen = ParentRegion == OMPD_unknown; Recommend = ShouldBeInTargetRegion; } else if (CurrentRegion == OMPD_scan) { if (SemaRef.LangOpts.OpenMP >= 50) { - SmallVector<OpenMPDirectiveKind, 4> LeafOrComposite; - std::ignore = getLeafOrCompositeConstructs(ParentRegion, LeafOrComposite); // OpenMP spec 5.0 and 5.1 require scan to be directly enclosed by for, // simd, or for simd. This has to take into account combined directives. // In 5.2 this seems to be implied by the fact that the specified // separated constructs are do, for, and simd. - OpenMPDirectiveKind Enclosing = LeafOrComposite.back(); - NestingProhibited = Enclosing != OMPD_for && Enclosing != OMPD_simd && - Enclosing != OMPD_for_simd; + NestingProhibited = !llvm::is_contained( + {OMPD_for, OMPD_simd, OMPD_for_simd}, EnclosingConstruct); } else { NestingProhibited = true; } OrphanSeen = ParentRegion == OMPD_unknown; Recommend = ShouldBeInLoopSimdRegion; } + if (!NestingProhibited && !isOpenMPTargetExecutionDirective(CurrentRegion) && !isOpenMPTargetDataManagementDirective(CurrentRegion) && - (ParentRegion == OMPD_teams || ParentRegion == OMPD_target_teams)) { + EnclosingConstruct == OMPD_teams) { // OpenMP [5.1, 2.22, Nesting of Regions] // distribute, distribute simd, distribute parallel worksharing-loop, // distribute parallel worksharing-loop SIMD, loop, parallel regions, @@ -5024,24 +5021,25 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, CurrentRegion == OMPD_atomic); Recommend = ShouldBeInParallelRegion; } + if (!NestingProhibited && CurrentRegion == OMPD_loop) { // OpenMP [5.1, 2.11.7, loop Construct, Restrictions] // If the bind clause is present on the loop construct and binding is // teams then the corresponding loop region must be strictly nested inside // a teams region. - NestingProhibited = BindKind == OMPC_BIND_teams && - ParentRegion != OMPD_teams && - ParentRegion != OMPD_target_teams; + NestingProhibited = + BindKind == OMPC_BIND_teams && EnclosingConstruct != OMPD_teams; Recommend = ShouldBeInTeamsRegion; } + if (!NestingProhibited && isOpenMPNestingDistributeDirective(CurrentRegion)) { // OpenMP 4.5 [2.17 Nesting of Regions] // The region associated with the distribute construct must be strictly // nested inside a teams region - NestingProhibited = - (ParentRegion != OMPD_teams && ParentRegion != OMPD_target_teams); + NestingProhibited = EnclosingConstruct != OMPD_teams; Recommend = ShouldBeInTeamsRegion; } + if (!NestingProhibited && (isOpenMPTargetExecutionDirective(CurrentRegion) || isOpenMPTargetDataManagementDirective(CurrentRegion))) { @@ -5061,6 +5059,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, false /* don't skip top directive */); CloseNesting = false; } + if (NestingProhibited) { if (OrphanSeen) { SemaRef.Diag(StartLoc, diag::err_omp_orphaned_device_directive) >From cdfcbc0605148f77470bc6269dea4e997d77fbcb Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <krzysztof.parzys...@amd.com> Date: Thu, 11 Jul 2024 07:59:30 -0500 Subject: [PATCH 3/3] Remove empty lines --- clang/lib/Sema/SemaOpenMP.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 5861923582eb1..7ca89b0d4eb70 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -4811,7 +4811,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, << getOpenMPDirectiveName(CurrentRegion); return true; } - if (isOpenMPSimdDirective(ParentRegion) && ((SemaRef.LangOpts.OpenMP <= 45 && CurrentRegion != OMPD_ordered) || (SemaRef.LangOpts.OpenMP >= 50 && CurrentRegion != OMPD_ordered && @@ -4835,14 +4834,12 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, << (SemaRef.LangOpts.OpenMP >= 50 ? 1 : 0); return CurrentRegion != OMPD_simd; } - if (EnclosingConstruct == OMPD_atomic) { // OpenMP [2.16, Nesting of Regions] // OpenMP constructs may not be nested inside an atomic region. SemaRef.Diag(StartLoc, diag::err_omp_prohibited_region_atomic); return true; } - if (CurrentRegion == OMPD_section) { // OpenMP [2.7.2, sections Construct, Restrictions] // Orphaned section directives are prohibited. That is, the section @@ -4856,7 +4853,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, } return false; } - // Allow some constructs (except teams and cancellation constructs) to be // orphaned (they could be used in functions, called from OpenMP regions // with the required preconditions). @@ -4865,7 +4861,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, CurrentRegion != OMPD_cancellation_point && CurrentRegion != OMPD_cancel && CurrentRegion != OMPD_scan) return false; - // Checks needed for mapping "loop" construct. Please check mapLoopConstruct // for a detailed explanation if (SemaRef.LangOpts.OpenMP >= 50 && CurrentRegion == OMPD_loop && @@ -4878,7 +4873,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, << getOpenMPDirectiveName(CurrentRegion); return true; } - if (CurrentRegion == OMPD_cancellation_point || CurrentRegion == OMPD_cancel) { // OpenMP [2.16, Nesting of Regions] @@ -5001,7 +4995,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, OrphanSeen = ParentRegion == OMPD_unknown; Recommend = ShouldBeInLoopSimdRegion; } - if (!NestingProhibited && !isOpenMPTargetExecutionDirective(CurrentRegion) && !isOpenMPTargetDataManagementDirective(CurrentRegion) && EnclosingConstruct == OMPD_teams) { @@ -5021,7 +5014,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, CurrentRegion == OMPD_atomic); Recommend = ShouldBeInParallelRegion; } - if (!NestingProhibited && CurrentRegion == OMPD_loop) { // OpenMP [5.1, 2.11.7, loop Construct, Restrictions] // If the bind clause is present on the loop construct and binding is @@ -5031,7 +5023,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, BindKind == OMPC_BIND_teams && EnclosingConstruct != OMPD_teams; Recommend = ShouldBeInTeamsRegion; } - if (!NestingProhibited && isOpenMPNestingDistributeDirective(CurrentRegion)) { // OpenMP 4.5 [2.17 Nesting of Regions] // The region associated with the distribute construct must be strictly @@ -5039,7 +5030,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, NestingProhibited = EnclosingConstruct != OMPD_teams; Recommend = ShouldBeInTeamsRegion; } - if (!NestingProhibited && (isOpenMPTargetExecutionDirective(CurrentRegion) || isOpenMPTargetDataManagementDirective(CurrentRegion))) { @@ -5059,7 +5049,6 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack, false /* don't skip top directive */); CloseNesting = false; } - if (NestingProhibited) { if (OrphanSeen) { SemaRef.Diag(StartLoc, diag::err_omp_orphaned_device_directive) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits