[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:1877
+
+      Value *InverseSecondOp = Builder.CreateFNeg(SecondOperand);
+      replaceOperand(*II, NegatedOperandArg, OpNotNeg);
----------------
fhahn wrote:
> If I read the code correctly, this may not be the second operand but could also the first one if the second one is negated?
I think it might be better to name them 
1. NegatedOperand
2. OtherOperand or NonNegatedOperand


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1878
+      Value *InverseSecondOp = Builder.CreateFNeg(SecondOperand);
+      replaceOperand(*II, NegatedOperandArg, OpNotNeg);
+      replaceOperand(*II, SecondOperandArg, InverseSecondOp);
----------------
fhahn wrote:
> I thought an earlier version created a new call here, rather than updating the exist one. Did we agree that `replaceOperand` here is the right thing to do?
We agreed in using `replaceOperand` since it is used in other areas of the same file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133300