[PATCH] D102698: [InstCombine] Relaxed constraints of uses for exp(X) * exp(Y) -> exp(X + Y) and exp2(X) * exp2(Y) -> exp2(X + Y)

[Daniil Seredkin via Phabricator via llvm-commits]

vdsered added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/Local.h:470
+/// use
+bool isRestrictiveUseConstraintMet(Instruction *I);
+
----------------
efriedma wrote:
> Don't expose isRestrictiveUseConstraintMet in the header if you don't plan to use it anywhere.
I saw how a check like Op0->hasOneUse() || Op1->hasOneUse() is heavily used as a precondition for many patterns and I thought of making it as a helper here because isOnlyUserOfAnyOperand uses it too. However, there are also places where one writes something like match(Op0, m_OneUse(...)) and it can probably be a little bit better in terms of readability. So, I'm not sure if we actually need this function. Removed it here as it is not that important for the goal of the patch.


================
Comment at: llvm/include/llvm/Transforms/Utils/Local.h:474
+/// met or all of the operands are the same instruction
+bool isRelaxedUseConstraintMet(Instruction *I);
+
----------------
efriedma wrote:
> I don't like the name isRelaxedUseConstraintMet; doesn't provide any indication what it means at first glance. Maybe isOnlyUserOfAnyOperand?
Your suggestion is a good alternative to the previous name. Renamed it.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:3311
+         (OpB->get()->hasNUses(NumOps) &&
+          std::equal(++OpB, I->op_end(), I->op_begin()));
+}
----------------
efriedma wrote:
> llvm::is_splat?
Fixed.


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

https://reviews.llvm.org/D102698