[PATCH] D155218: [InstCombine] Optimize addition/subtraction operations of splats of vscale multiplied by a constant

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1529
+  // where B and C are splats of VScale multiplied by a number
+  if (Opcode == Instruction::Add || Opcode == Instruction::Sub) {
+    Value *A, *B, *C;
----------------
paulwalker-arm wrote:
> This function seems to handle cases where the opcode is not relevant.  However, you only care about two specific opcodes so this doesn't looks like the correct resting place for this code.  Placing it somewhere more specific to `add` and `sub` might allow you to simplify the logic.
Although you could imagine extending this function for any assosiative binop, i.e xor, or, and, mul...
Probably cleaner would to be split this to a helper function so that if you don't have supported opcodes you cna just return `nullptr`. Then there won't be so much nesting.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1554
+          Instruction::BinaryOps NewOpcode2;
+          if (IsBPositive && IsCPositive) {
+            NewOpcode1 = Instruction::Add;
----------------
Can you add a comment explaining this. Its not exactly clear how/why the positive bools impl the opcodes they do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155218