[PATCH] D112717: [IR] Replace *all* uses of a constant expression by corresponding instruction

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D112717#3702269 <https://reviews.llvm.org/D112717#3702269>, @JonChesterfield wrote:

> This doesn't work with phi nodes that have multiple uses of the same constantexpr. Specifically it creates replacement instructions for that constantexpr and inserts them on the earliest incoming edge that uses that constantexpr, then replaces all uses with the new instruction. That works iff the earliest incoming edge happens to dominate the other incoming edges that use the same constantexpr, otherwise it emits broken IR.
>
> I can't work out how this is supposed to work. One phase builds a collectConstantExprPaths object and a second uses it. I think the error is in the call to `II->replaceUsesOfWith(CE, NI)`. As a workaround I'm currently using the following as a replacement for these functions which seems to be fine so far but will probably fail the test cases written for the above.
>
>   void eliminateConstantExprsFromInstruction(Instruction *I) {
>     SmallVector<Instruction *> Worklist{I};
>   
>     while (!Worklist.empty()) {
>       Instruction *I = Worklist.pop_back_val();
>       for (Use &U : I->operands()) {
>   
>         auto *BI = I;
>         if (auto *Phi = dyn_cast<PHINode>(I)) {
>           BasicBlock *BB = Phi->getIncomingBlock(U);
>           BI = &(*(BB->getFirstInsertionPt()));
>         }
>   
>         if (ConstantExpr *C = dyn_cast<ConstantExpr>(U.get())) {
>           Instruction *NI = C->getAsInstruction(BI);
>           Worklist.push_back(NI);
>           U.set(NI);
>           C->removeDeadConstantUsers();
>         }
>       }
>     }
>   }

With the plan to drop most constant expressions, could we start opting LDS globals out of constant expressions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112717