[PATCH] D133300: [InstCombine] Matrix multiplication negation optimisation

[Zain Jaffal via Phabricator via llvm-commits]

zjaffal added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1850-1858
+    bool MatchOp0 = match(Op0, m_FNeg(m_Value(X)));
+    bool MatchOp1 = match(Op1, m_FNeg(m_Value(X)));
+    if (MatchOp0 && MatchOp1) {
+      Instruction *FNegOp0 = cast<Instruction>(Op0);
+      Instruction *FNegOp1 = cast<Instruction>(Op1);
+      replaceInstUsesWith(*FNegOp0, FNegOp0->getOperand(0));
+      replaceInstUsesWith(*FNegOp1, FNegOp1->getOperand(0));
----------------
spatel wrote:
> I don't think this is correct if an fneg has multiple uses (similar to the bug noted earlier, and I repeat my suggestion to create new instructions rather than modifying existing ones).
> 
> Please split this change and tests to its own review ahead of the original transforms in this patch. 
We don't need this check anyways. 
if both operands are negative then there is two cases:
1. Both of the negations will be moved to the result and then another pass will remove the negations
2. negation gets moved to an operand and then we have two negations on one operand which will be optimised as well.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133300