[PATCH] D142293: [InstCombine] Add PHINode operands to worklist on instruction erasure

[Luke Lau via Phabricator via llvm-commits]

luke added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineInternal.h:423
+            Worklist.add(PN);
+      }
 
----------------
Will this cause much extra work? 
It certainly improves the pathological case in https://github.com/llvm/llvm-project/issues/50564, but not sure how this affects day to day cases.

The PHI node elimination that kicks in is at InstCombinePHI.cpp:1439

```cpp
 // If this phi has a single use, and if that use just computes a value for 
 // the next iteration of a loop, delete the phi.  This occurs with unused 
 // induction variables, e.g. "for (int j = 0; ; ++j);".  Detecting this 
 // common case here is good because the only other things that catch this 
 // are induction variable analysis (sometimes) and ADCE, which is only run 
 // late. 
 if (PHIUser->hasOneUse() && 
     (isa<BinaryOperator>(PHIUser) || isa<GetElementPtrInst>(PHIUser)) && 
     PHIUser->user_back() == &PN) { 
   return replaceInstUsesWith(PN, PoisonValue::get(PN.getType())); 
 } 
```

Another possible idea could be to somehow invert this and instead check on every instruction if it has one use that's a phi node.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142293