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

[Sanjay Patel via Phabricator via llvm-commits]

spatel 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));
----------------
fhahn wrote:
> zjaffal wrote:
> > 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.
> > 
> @zjaffal do we have a test case where both operands are negated and at least one of the fnegs has multiple uses?
Correct me if I'm wrong:
1. When both ops are negated, we don't care what the relative sizes of the values are, we always want to use the non-negated source ops.
2. When both ops are negated, we don't care if those ops have other uses, we always want to use the non-negated source ops.

Either way, we need tests to exercise those patterns.

```

define <4 x double> @matrix_multiply_v2f64_v2f64(<2 x double> %a, <2 x double> %b) {
  %a.neg = fneg <2 x double> %a
  %b.neg = fneg <2 x double> %b
  %res = call <4 x double> @llvm.matrix.multiply.v4f64.v2f64.v2f64(<2 x double> %a.neg, <2 x double> %b.neg, i32 2, i32 1, i32 2)
  ret <4 x double> %res
}

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133300