Skip to content

LoopDeletion: Move EH pad check before the isLoopNeverExecuted Check #78189

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
Jan 19, 2024

Conversation

Nirhar
Copy link
Contributor

@Nirhar Nirhar commented Jan 15, 2024

This commit modifies LoopDeletion::deleteLoopIfDead to check if the exit block of a loop is an EH pad before checking if the loop gets executed. This handles the case where an unreacheable loop has a landingpad as an Exit block, and the loop gets deleted, leaving leaving the landingpad without an edge from an unwind clause.

Fixes #76852

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@Nirhar
Copy link
Contributor Author

Nirhar commented Jan 15, 2024

@nikic Have a look at these changes, thanks!

@llvmbot
Copy link
Member

llvmbot commented Jan 15, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Manish Kausik H (Nirhar)

Changes

This commit modifies LoopDeletion::deleteLoopIfDead to check if the exit block of a loop is an EH pad before checking if the loop gets executed. This handles the case where an unreacheable loop has a landingpad as an Exit block, and the loop gets deleted, leaving leaving the landingpad without an edge from an unwind clause.

Fixes #76852


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/LoopDeletion.cpp (+7-7)
diff --git a/llvm/lib/Transforms/Scalar/LoopDeletion.cpp b/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
index c041e3621a16bd..bfe9374cf2f8c8 100644
--- a/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
@@ -452,6 +452,13 @@ static LoopDeletionResult deleteLoopIfDead(Loop *L, DominatorTree &DT,
 
   BasicBlock *ExitBlock = L->getUniqueExitBlock();
 
+  // We can't directly branch to an EH pad. Don't bother handling this edge
+  // case.
+  if (ExitBlock && ExitBlock->isEHPad()) {
+    LLVM_DEBUG(dbgs() << "Cannot delete loop exiting to EH pad.\n");
+    return LoopDeletionResult::Unmodified;
+  }
+
   if (ExitBlock && isLoopNeverExecuted(L)) {
     LLVM_DEBUG(dbgs() << "Loop is proven to never execute, delete it!\n");
     // We need to forget the loop before setting the incoming values of the exit
@@ -487,13 +494,6 @@ static LoopDeletionResult deleteLoopIfDead(Loop *L, DominatorTree &DT,
     return LoopDeletionResult::Unmodified;
   }
 
-  // We can't directly branch to an EH pad. Don't bother handling this edge
-  // case.
-  if (ExitBlock && ExitBlock->isEHPad()) {
-    LLVM_DEBUG(dbgs() << "Cannot delete loop exiting to EH pad.\n");
-    return LoopDeletionResult::Unmodified;
-  }
-
   // Finally, we have to check that the loop really is dead.
   bool Changed = false;
   if (!isLoopDead(L, SE, ExitingBlocks, ExitBlock, Changed, Preheader, LI)) {

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Please also add a test case in llvm/test/Transforms/LoopDeletion.

@Nirhar Nirhar force-pushed the loop-deletion-error branch from 98a6318 to 4d8faa9 Compare January 18, 2024 16:32
@Nirhar
Copy link
Contributor Author

Nirhar commented Jan 18, 2024

@nikic Have a look at the added test , thanks!

This commit modifies `LoopDeletion::deleteLoopIfDead` to check if
the exit block of a loop is an EH pad before checking if the loop
gets executed. This handles the case where an unreacheable loop has
a landingpad as an Exit block, and the loop gets deleted, leaving
leaving the landingpad without an edge from an unwind clause.

Fixes llvm#76852
@Nirhar Nirhar force-pushed the loop-deletion-error branch from 4d8faa9 to c2f5d0b Compare January 18, 2024 18:52
@Nirhar Nirhar requested a review from nikic January 18, 2024 18:53
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@nikic nikic merged commit a0b9117 into llvm:main Jan 19, 2024
@Nirhar Nirhar deleted the loop-deletion-error branch January 28, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLVM ERROR: Broken module found, compilation aborted!
3 participants