[PATCH] D105392: [InstCombine] Add optimization to prevent poison from being propagated.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Almost there i think



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3532-3533
+  auto* OrigOp = OrigFI.getOperand(0);
+  if (auto *OrigOpInst = dyn_cast<Instruction>(OrigOp)) {
+    if (OrigOpInst->hasOneUse() && !canCreateUndefOrPoison(dyn_cast<Operator>(OrigOp))) {
+      Optional<Use *> MaybePoisonOperand;
----------------
Early return please
Also, this needs a comment why we don't want to just adjust all the other uses.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3534
+    if (OrigOpInst->hasOneUse() && !canCreateUndefOrPoison(dyn_cast<Operator>(OrigOp))) {
+      Optional<Use *> MaybePoisonOperand;
+      for (Use &U : OrigOpInst->operands()) {
----------------
Ok, `Optional` isn't much better here than just storing a pointer, let's go back to that one.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3540-3541
+          MaybePoisonOperand = &U;
+        else if(MaybePoisonOperand)
+          return nullptr;
+      }
----------------



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3544-3545
+
+      if (!MaybePoisonOperand.hasValue())
+        return nullptr;
+
----------------
Can we just drop `freeze` then?


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3548
+      auto *MaybePoisonUse = MaybePoisonOperand.getValue();
+      auto *FrozenMaybePoisonOperand = new FreezeInst(MaybePoisonUse->get(), MaybePoisonUse->getUser()->getName() + ".fr");
+
----------------
Not clang-formatted


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105392