Skip to content

[OMPIRBuilder] Don't discard the debug record from entry block. #135161

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 3 commits into from
Apr 30, 2025

Conversation

abidh
Copy link
Contributor

@abidh abidh commented Apr 10, 2025

When we get a function back from CodeExtractor, we discard its entry block after coping its instructions into the entry block we prepared. While copying the instructions, the terminator is discarded for obvious reasons. But if there were some debug values attached to the terminator, those are useful and needs to be copied.

@llvmbot llvmbot added flang:openmp clang:openmp OpenMP related changes to Clang labels Apr 10, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2025

@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-flang-openmp

Author: Abid Qadeer (abidh)

Changes

When we get a function back from CodeExtractor, we discard its entry block after coping its instructions into the entry block we prepared. While copying the instructions, the terminator is discarded for obvious reasons. But if there were some debug values attached to the terminator, those are useful and needs to be copied.


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

1 Files Affected:

  • (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+6-1)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 28662efc02882..f946fab42ca7a 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -788,8 +788,13 @@ void OpenMPIRBuilder::finalize(Function *Fn) {
         Instruction &I = *It;
         It++;
 
-        if (I.isTerminator())
+        if (I.isTerminator()) {
+          // Absorb any debug value that terminator may have
+          if (OI.EntryBB->getTerminator())
+            OI.EntryBB->getTerminator()->adoptDbgRecords(
+                &ArtificialEntry, I.getIterator(), false);
           continue;
+        }
 
         I.moveBeforePreserving(*OI.EntryBB, OI.EntryBB->getFirstInsertionPt());
       }

Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

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

Could you add a test so we do not regress in the future?

@abidh
Copy link
Contributor Author

abidh commented Apr 14, 2025

Could you add a test so we do not regress in the future?

Hi @Meinersbur

Thanks for taking a look. Although this change fixes a missing piece, there is currently no way to test it as CodeExtractor does not generate the debug info in the entry block. This is preparatory PR for the bigger changes I am working on to improve the debug info of the function generated by the CodeExtractor. The bigger PR will have the tests.

This piece was independent and self contained so I thought I will post it separately.

abidh added a commit to abidh/llvm-project that referenced this pull request Apr 15, 2025
When we get a function back from CodeExtractor, we disard its entry block after coping its instructions into the entry block we prepared. While copying the instructions, the terminator is discarded for obvious reasons. But if there were some debug values attached to the terminator, those are useful and needs to be copied.

llvm#135161
abidh added 3 commits April 26, 2025 11:35
When we get a function back from CodeExtractor, we disard its entry
block after coping its instructions into the entry block we prepared.
While copying the instructions, the terminator is discarded for obvious
reasons. But if there were some debug values attached to the terminator,
those are useful and needs to be copied.
@llvmbot llvmbot added clang Clang issues not falling into any other category mlir:llvm mlir labels Apr 28, 2025
@abidh
Copy link
Contributor Author

abidh commented Apr 28, 2025

We can test this functionality now as #136016 has been merged. I have added a test and updated relevant clang tests.

@abidh abidh requested a review from Meinersbur April 28, 2025 16:47
Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@abidh abidh merged commit 04aa5a8 into llvm:main Apr 30, 2025
14 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…#135161)

When we get a function back from `CodeExtractor`, we discard its entry
block after coping its instructions into the entry block we prepared.
While copying the instructions, the terminator is discarded for obvious
reasons. But if there were some debug values attached to the terminator,
those are useful and needs to be copied.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…#135161)

When we get a function back from `CodeExtractor`, we discard its entry
block after coping its instructions into the entry block we prepared.
While copying the instructions, the terminator is discarded for obvious
reasons. But if there were some debug values attached to the terminator,
those are useful and needs to be copied.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…#135161)

When we get a function back from `CodeExtractor`, we discard its entry
block after coping its instructions into the entry block we prepared.
While copying the instructions, the terminator is discarded for obvious
reasons. But if there were some debug values attached to the terminator,
those are useful and needs to be copied.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…#135161)

When we get a function back from `CodeExtractor`, we discard its entry
block after coping its instructions into the entry block we prepared.
While copying the instructions, the terminator is discarded for obvious
reasons. But if there were some debug values attached to the terminator,
those are useful and needs to be copied.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…#135161)

When we get a function back from `CodeExtractor`, we discard its entry
block after coping its instructions into the entry block we prepared.
While copying the instructions, the terminator is discarded for obvious
reasons. But if there were some debug values attached to the terminator,
those are useful and needs to be copied.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category flang:openmp mlir:llvm mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants