[llvm] [SLP] Check the Operands of Copyable elements as well in getBestOperand() (PR #182443)

[Alexey Bataev via llvm-commits]

================
@@ -2996,16 +2998,39 @@ class slpvectorizer::BoUpSLP {
         case ReorderingMode::Load:
         case ReorderingMode::Opcode: {
           bool LeftToRight = Lane > LastLane;
-          Value *OpLeft = (LeftToRight) ? OpLastLane : Op;
-          Value *OpRight = (LeftToRight) ? Op : OpLastLane;
-          int Score = getLookAheadScore(OpLeft, OpRight, MainAltOps, Lane,
-                                        OpIdx, Idx, IsUsed, UsedLanes);
-          if (Score > static_cast<int>(BestOp.Score) ||
-              (Score > 0 && Score == static_cast<int>(BestOp.Score) &&
-               Idx == OpIdx)) {
-            BestOp.Idx = Idx;
-            BestOp.Score = Score;
-            BestScoresPerLanes[std::make_pair(OpIdx, Lane)] = Score;
+          auto ScoreLoadOrOpcode = [&](Value *Op, Value *OpLastLane) -> void {
+            Value *OpLeft = (LeftToRight) ? OpLastLane : Op;
+            Value *OpRight = (LeftToRight) ? Op : OpLastLane;
+            int Score = getLookAheadScore(OpLeft, OpRight, MainAltOps, Lane,
+                                          OpIdx, Idx, IsUsed, UsedLanes);
+            if (Score > static_cast<int>(BestOp.Score) ||
+                (Score > 0 && Score == static_cast<int>(BestOp.Score) &&
+                 Idx == OpIdx)) {
+              BestOp.Idx = Idx;
+              BestOp.Score = Score;
+              BestScoresPerLanes[std::make_pair(OpIdx, Lane)] = Score;
+            }
+          };
+          ScoreLoadOrOpcode(Op, OpLastLane);
+          // If Op or OpLastLane is Copyable, score against the Copyable Value's
+          // operands
+          if (S.isCopyableElement(VL[Lane]) !=
+              S.isCopyableElement(VL[LastLane])) {
+            bool OpIsCopyable = S.isCopyableElement(VL[Lane]);
+            Instruction *I =
+                dyn_cast<Instruction>(OpIsCopyable ? Op : OpLastLane);
+            if (I && Instruction::isBinaryOp(I->getOpcode())) {
+              Value *IOp0 = I->getOperand(0);
+              Value *NewOp = OpIsCopyable ? IOp0 : Op;
+              Value *NewOpLastLane = OpIsCopyable ? OpLastLane : IOp0;
+              ScoreLoadOrOpcode(NewOp, NewOpLastLane);
+              if (I->isCommutative()) {
+                Value *IOp1 = I->getOperand(1);
+                NewOp = OpIsCopyable ? IOp1 : Op;
+                NewOpLastLane = OpIsCopyable ? OpLastLane : IOp1;
+                ScoreLoadOrOpcode(NewOp, NewOpLastLane);
+              }
+            }
----------------
alexey-bataev wrote:

After some thought, I don't quite understand why we need this special matching against copyables. It should not matter if it is a copyable value or not; operands are already built, and we should match the operands against each other, as if there were no copyables at all.
If we see that some operands are reordered properly, it means we have an issue in the cost algorithm, which should not depend on a copyable root instruction.

https://github.com/llvm/llvm-project/pull/182443