[PATCH] D98378: [LSR] RFC: Fix crash in Phi node with EHPad block

[Lorenz Brun via Phabricator via llvm-commits]

lorenz created this revision.
Herald added a subscriber: hiraditya.
lorenz requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This fixes a crash I observed in issue #48708 where the LSR
pass tries to insert an instruction in a basic block with only a
`catchswitch` statement in there. This happens because the Phi node
being evaluated has two incoming values which are reference-equal
but different basic blocks.

If the basic block associated with the incoming value of the operand
being evaluated has an EHPad terminator LSR drops that in the check
just above the one I added. But if that incoming value is reference-
equal to another incoming value, RewriteForPhi ends up rewriting
both values, one of which can have an EHPad terminator and thus
hits the "Insertion point must be a normal instruction" assert in
`AdjustInsertPositionForExpand`.

This fix makes CollectLoopInvariantFixupsAndFormulae also ignore
cases where a reference-equal value has a basic block with an
EHPad, same as it already does in case the primary value has one.

Test Plan: I'll write once it's clear that this is the right solution.
A reproducer is in the associated issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98378

Files:
  llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp


Index: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -3444,6 +3444,26 @@
         // Don't bother if the instruction is in a BB which ends in an EHPad.
         if (UseBB->getTerminator()->isEHPad())
           continue;
+
+        // Check if a PHI node has reference-equal inputs which are terminated
+        // with non-splittable instructions
+        if (isa<PHINode>(UserInst)) {
+          const auto *PhiNode = cast<PHINode>(UserInst);
+          bool HasIncompatibleEHPTerminatedBlock = false;
+          llvm::Value *ExpectedValue = U;
+          for (unsigned int I = 0; I < PhiNode->getNumIncomingValues(); I++) {
+            if (PhiNode->getIncomingValue(I) == ExpectedValue) {
+              if (PhiNode->getIncomingBlock(I)->getTerminator()->isEHPad()) {
+                HasIncompatibleEHPTerminatedBlock = true;
+                break;
+              }
+            }
+          }
+          if (HasIncompatibleEHPTerminatedBlock) {
+            continue;
+          }
+        }
+
         // Don't bother rewriting PHIs in catchswitch blocks.
         if (isa<CatchSwitchInst>(UserInst->getParent()->getTerminator()))
           continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98378.329803.patch
Type: text/x-patch
Size: 1343 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210311/d1915e29/attachment.bin>