[PATCH] D158079: [InstCombine] Contracting x^2 + 2*x*y + y^2 to (x + y)^2 (float)

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1018
+          unsigned Mul2Op =
+              conditional_value<FP, Instruction::FMul, Instruction::Shl>::value>
+static bool matchesSquareSum(BinaryOperator &I, Mul2Rhs M2Rhs, Value *&A,
----------------
In fact if you do the bool template approach, I don't think `MulOp`, `AddOp`, or `Mul2Op` even need to be template parameters. You can just set them as unsigned values at the top of `matchesSquareSum`.
i.e: `unsigned MulOp = FP ? Instruction::FMul : Instruction::Mul;`
Imo that ends up being cleaner.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1050
+  Value *A, *B;
+  if (matchesSquareSum<false>(I, m_SpecificInt(1), A, B)) {
     Value *AB = Builder.CreateAdd(A, B);
----------------
comment what `false` means. I.e `</*FP*/false>`


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1062
+  Value *A, *B;
+  if (matchesSquareSum<true>(I, m_SpecificFP(2.0), A, B)) {
+    Value *AB = Builder.CreateFAddFMF(A, B, &I);
----------------
ibid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158079