Page MenuHomePhabricator

[flang][OpenMP] Add functionality to check "close nesting" of regions, which can be used for Semantic checks.
ClosedPublic

Authored by arnamoy10 on Tue, Apr 6, 9:43 AM.

Details

Summary

This patch adds a function to help Semantic checks that deal with "closely nesting" condition check.

Also modifies the closely nesting semantic check code for worksharing regions. Previously the check was checking only the immediate parent. e.g

!$omp do
 !omp target
  !$omp do

In the previous code, though the inside do is closely nested inside the outside do (no parallel in between), the existing check would not catch the violation, as the immediate parent of the nested do is target.

With the new function, this condition will be caught.

More test cases are added to check more scenarios.

Diff Detail

Event Timeline

arnamoy10 created this revision.Tue, Apr 6, 9:43 AM
arnamoy10 requested review of this revision.Tue, Apr 6, 9:43 AM
Herald added a project: Restricted Project. · View Herald Transcript
flang/lib/Semantics/check-omp-structure.cpp
130

I was thinking you will be doing the following. Is there an issue with this approach?

  1. curIndex = index of top of stack
  2. curDir = Directive at curIndex
  3. If (curDir is a violating nesting) return true
  4. if (curDir is parallel) return false
  5. if curIndex == -1 return false
  6. curIndex = curIndex - 1
  7. goto step 2
flang/lib/Semantics/check-omp-structure.h
169

Can you change the name to IsCloselyNestedRegion.

Note that the definition of closely nested construct is different. A construct nested inside another construct with no other construct nested between them.

arnamoy10 added inline comments.Tue, Apr 6, 11:31 AM
flang/lib/Semantics/check-omp-structure.cpp
130

I do not think I understand Step 3: If (curDir is a violating nesting) return true

Let's say we are doing the violation check that barrier is not allowed inside a task

$!omp task
  $!omp target
    $!omp barrier --> top of stack

So curDir is barrier. Could you tell me how step 3 would work as per your algorithm?

flang/lib/Semantics/check-omp-structure.cpp
130

In the algorithm I was suggesting the assumption I made is that the top of stack is !$omp target and not !$omp barrier.

arnamoy10 updated this revision to Diff 335787.Wed, Apr 7, 5:54 AM
  1. Updating the algorithm as per suggestion
  2. Modified the name of the function to IsCloselyNestedRegion()
flang/lib/Semantics/check-omp-structure.cpp
130

That makes sense, thank you. I think the suggested algorithm is cleaner and I updated the patch with the implementation

kiranchandramohan accepted this revision.Thu, Apr 8, 8:51 AM

LGTM. Thanks for the changes and the tests.

This revision is now accepted and ready to land.Thu, Apr 8, 8:51 AM
This revision was landed with ongoing or failed builds.Thu, Apr 8, 12:17 PM
This revision was automatically updated to reflect the committed changes.