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

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm 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;
----------------
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.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1534
+    if (!C || !isConstantMultipliedVScale(C)) {
+      C = getSplatValue(RHS);
+      Nested = dyn_cast<Instruction>(RHS);
----------------
Should this be `C = getSplatValue(LHS);`? if so then perhaps this highlights some missing tests?


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1550-1551
+          // Deduce the new opcode based on the positivity of splat operations
+          auto IsBPositive = Opcode2 == Instruction::Add;
+          auto IsCPositive = Opcode == Instruction::Add;
+          Instruction::BinaryOps NewOpcode1;
----------------
Not sure this naming is correct because the opcode says nothing about the signedness of the data.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155218