[PATCH] D105309: [InstCombine] Don't combine PHI before catchswitch

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:1331
     CheckedIVs.insert(IV0);
     if (IV0 != IV0Stripped &&
         all_of(PN.incoming_values(), [&CheckedIVs, IV0Stripped](Value *IV) {
----------------
Every other return path of `InstCombinerImpl::visitPHINode` appears to create and return an existing instruction of a new phi node. This one is special in that it creates a new non-phi instruction.

The insertion location is not determined here, but it is predictable, it's the first available insertion point after the phi, but the phi's block has no insertion point. The generic way to test for this is to compare `getInsertionPoint` against the end iterator.

So, all the other transforms seem fine. It's reasonable to put a more targeted fix right here: 
  && PN.getParent()->getFirstNonPHI() != PN.getParent()->end()


================
Comment at: llvm/test/Transforms/InstCombine/catchswitch-phi.ll:1
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1"
+target triple = "wasm32-unknown-unknown"
----------------
lebedev.ri wrote:
> This isn't testing anything
That's true, and I don't see any cast instructions to trigger the codepath we are looking at.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105309