[PATCH] D106166: [LV][ARM] Tighten up MLA reduction costing

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7193
+            m_ZExtOrSExt(m_Mul(m_Instruction(Op0), m_Instruction(Op1)))) &&
+      match(Op0, m_ZExtOrSExt(m_Value())) &&
+      Op0->getOpcode() == Op1->getOpcode() &&
----------------
Do we also need to match `op1`?

  match(Op1, m_ZExtOrSExt(m_Value())

that's what I would guess from reading the comment below.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7198
+      (Op0->getOpcode() == RedOp->getOpcode() || Op0 == Op1)) {
+    // Matched reduce(ext(mul(ext(A), ext(B)))
+    bool IsUnsigned = isa<ZExtInst>(Op0);
----------------
Nit: might be easier to read if this comment is just before the if. 


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7212
+
+    if (RedCost.isValid() && RedCost < ExtCost * 2 + MulCost + BaseCost)
+      return I == RetI ? RedCost : 0;
----------------
Just a quick query on the  `* 2`, was wondering if that needs to be 3, but probably depends on my earlier question about matching op1.


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

https://reviews.llvm.org/D106166