[PATCH] D114647: [TrivialDeadness] Introduce API separating two different usages

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

I think most of this logic really belongs to `isGuaranteedToTransferExecutionToSuccessor`. I also think that `wouldInstructionBeTriviallyDeadOnPathsWithoutUse` should simply be `isGuaranteedToTransferExecutionToSuccessor && !mayHaveSideEffects`, unless you have a counter-test.



================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:447
+  if (auto *Call = dyn_cast<CallBase>(I))
+    if (isMathLibCallNoop(Call, TLI))
+      return true;
----------------
This also looks like it can be made a part of `isGuaranteedToTransferExecutionToSuccessor`.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:468
+    if ((II->getIntrinsicID() == Intrinsic::assume &&
+         isAssumeWithEmptyBundle(cast<AssumeInst>(*II))) ||
+        II->getIntrinsicID() == Intrinsic::experimental_guard) {
----------------
Please remove this check. You may think that `assume` always passes execution, because it is either a NOOP or entire program is undefined.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:471
+      if (ConstantInt *Cond = dyn_cast<ConstantInt>(II->getArgOperand(0)))
+        return !Cond->isZero();
+
----------------
This should really be a part of `isGuaranteedToTransferExecutionToSuccessor`. I suggest to check it instead of `I->willReturn()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114647