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

Quentin Colombet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 14 18:53:46 PST 2022


This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa8ca4046e22d: [LSR] Fix crash in Phi node with EHPad block (authored by qcolombet).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98378/new/

https://reviews.llvm.org/D98378

Files:
  llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
  llvm/test/Transforms/LoopStrengthReduce/phi_ehpad_ignore_sameval.ll


Index: llvm/test/Transforms/LoopStrengthReduce/phi_ehpad_ignore_sameval.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/LoopStrengthReduce/phi_ehpad_ignore_sameval.ll
@@ -0,0 +1,55 @@
+; This tests that LoopStrengthReduce ignores possible optimizations that are
+; not realizable because they would require rewriting EHPad-class instructions.
+; If this type of optimization is attempted it will hit the
+; "Insertion point must be a normal instruction" assertion.
+;
+; See also https://bugs.llvm.org/show_bug.cgi?id=48708
+;
+; RUN: opt -loop-reduce -S %s
+
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:6:2:64-S128"
+
+%"class.std::allocator" = type { i8 }
+%"class.absl::Storage" = type {}
+
+define void @0() personality i8* undef {
+init1:
+  %i14 = invoke i8* undef(i8* null, i8 0)
+          to label %init2 unwind label %unwind
+
+init2:                                            ; preds = %init1
+  %i19 = select i1 undef, %"class.std::allocator"* null, %"class.std::allocator"* null
+  br label %loop
+
+loop:                                             ; preds = %loop.increment, %init2
+  %i21 = phi i64 [ %i24, %loop.increment ], [ 0, %init2 ]
+  %i22 = getelementptr %"class.std::allocator", %"class.std::allocator"* %i19, i64 %i21
+  invoke void undef(i8* null, %"class.std::allocator"* null, %"class.std::allocator"* %i22)
+          to label %loop.increment unwind label %loop.unwind
+
+loop.increment:                                   ; preds = %loop
+  %i24 = add i64 %i21, 1
+  br label %loop
+
+loop.unwind:                                      ; preds = %loop
+  %i26 = catchswitch within none [label %loop.catch] unwind label %unwind
+
+loop.catch:                                       ; preds = %loop.unwind
+  %i28 = catchpad within %i26 []
+  catchret from %i28 to label %caught
+
+caught:                                           ; preds = %loop.catch
+  invoke void undef(%"class.absl::Storage"* null)
+          to label %unreach unwind label %unwind
+
+unreach:                                          ; preds = %caught
+  unreachable
+
+unwind:                                           ; preds = %caught, %loop.unwind, %init1
+  ; This phi node triggers the issue in combination with the optimizable loop
+  ; above. It contains %i19 twice, once from %caught (which doesn't have an
+  ; EHPad) and once from %loop.unwind, which does have one.
+  %i32 = phi %"class.std::allocator"* [ %i19, %loop.unwind ], [ %i19, %caught ], [ null, %init1 ]
+  %i33 = cleanuppad within none []
+  cleanupret from %i33 unwind to caller
+}
Index: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -3486,6 +3486,31 @@
         // Don't bother if the instruction is in a BB which ends in an EHPad.
         if (UseBB->getTerminator()->isEHPad())
           continue;
+
+        // Ignore cases in which the currently-examined value could come from
+        // a basic block terminated with an EHPad. This checks all incoming
+        // blocks of the phi node since it is possible that the same incoming
+        // value comes from multiple basic blocks, only some of which may end
+        // in an EHPad. If any of them do, a subsequent rewrite attempt by this
+        // pass would try to insert instructions into an EHPad, hitting an
+        // assertion.
+        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.400226.patch
Type: text/x-patch
Size: 4367 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220115/056f6215/attachment.bin>


More information about the llvm-commits mailing list