[PATCH] D142551: [InstCombine] Teach isDeadPHICycle to look through multiple uses

[Luke Lau via Phabricator via llvm-commits]

luke marked 3 inline comments as done.
luke added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:1023
+  // also in a cycle.
+  if (I->use_empty() || I->mayHaveSideEffects())
+    return false;
----------------
nikic wrote:
> Returning `false` for `I->use_empty()` doesn't really make sense. I suspect that what you're trying to work around here is the something like `br` doesn't have side effects, but also can't just be removed. The correct predicate to check here is `wouldInstructionBeTriviallyDead`.
That makes much more sense, thanks. 
I've updated the diff but now `isDeadPHICycle` is really just `isDeadCycle`, which makes me wonder if there's another, more generic place for this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142551