Skip to content

[Flang][OpenMP] Use InsertionGuard in DataSharingProcessor #97562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

skatrak
Copy link
Member

@skatrak skatrak commented Jul 3, 2024

This patch removes the introduction of fir.undef operations as a way to keep track of insertion points inside of the DataSharingProcessor, and it replaces them with an InsertionGuard to avoid creating such operations inside of loop wrappers.

Leaving any fir.undef operation inside of a loop wrapper would result in a verifier error, since they enforce strict requirements on the contents of their code regions.

This patch removes the introduction of `fir.undef` operations as a way to keep
track of insertion points inside of the `DataSharingProcessor`, and it replaces
them with an `InsertionGuard` to avoid creating such operations inside of loop
wrappers.

Leaving any `fir.undef` operation inside of a loop wrapper would result in a
verifier error, since they enforce strict requirements on the contents of their
code regions.
@llvmbot
Copy link
Member

llvmbot commented Jul 3, 2024

@llvm/pr-subscribers-flang-fir-hlfir

Author: Sergio Afonso (skatrak)

Changes

This patch removes the introduction of fir.undef operations as a way to keep track of insertion points inside of the DataSharingProcessor, and it replaces them with an InsertionGuard to avoid creating such operations inside of loop wrappers.

Leaving any fir.undef operation inside of a loop wrapper would result in a verifier error, since they enforce strict requirements on the contents of their code regions.


Full diff: https://github.com/llvm/llvm-project/pull/97562.diff

1 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+1-4)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index e2b55fcc64062..7df3905c29990 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -72,11 +72,8 @@ void DataSharingProcessor::processStep2(mlir::Operation *op, bool isLoop) {
     firOpBuilder.setInsertionPointAfter(op);
     insertDeallocs();
   } else {
-    // insert dummy instruction to mark the insertion position
-    mlir::Value undefMarker = firOpBuilder.create<fir::UndefOp>(
-        op->getLoc(), firOpBuilder.getIndexType());
+    mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
     insertDeallocs();
-    firOpBuilder.setInsertionPointAfter(undefMarker.getDefiningOp());
   }
 }
 

Copy link
Contributor

@kparzysz kparzysz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@skatrak skatrak merged commit 99f6ff9 into main Jul 4, 2024
10 checks passed
@skatrak skatrak deleted the users/skatrak/composite-lower-01-dsp-ig branch July 4, 2024 12:13
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
This patch removes the introduction of `fir.undef` operations as a way
to keep track of insertion points inside of the `DataSharingProcessor`,
and it replaces them with an `InsertionGuard` to avoid creating such
operations inside of loop wrappers.

Leaving any `fir.undef` operation inside of a loop wrapper would result
in a verifier error, since they enforce strict requirements on the
contents of their code regions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants