[llvm] [SLP] Fix misvectorization in commutative to non-commutative conversion (PR #185230)

[via llvm-commits]

https://github.com/tudinhh created https://github.com/llvm/llvm-project/pull/185230

**Summary**
Fixes a miscompilation where commutative operations (e.g., or, and, mul) with a left-hand side constant were incorrectly transformed into non-commutative operations (e.g., shl, sub).

**The Problem**
In BinOpSameOpcodeHelper::getOperand, when a constant is at Pos == 0, the helper was failing to swap operand order for new non-commutative target opcodes. This resulted in inverted logic, such as transforming `or 0, %x` into `shl 0, %x` (resulting in 0) instead of the correct `%x << 0`.

**The Fix**
The existing logic only protected the Sub opcode. This patch generalizes the fix to all non-commutative instructions by using !Instruction::isCommutative(ToOpcode). This ensures that for any directional operation, the variable is correctly placed on the LHS and the constant on the RHS.

**Changes**
SLPVectorizer.cpp: Replaced the specific Sub check with a general isCommutative check.

Regression Test: Added lhs-constant-non-cummutative.ll to cover shl, sub, and ashr targets.

Fixes #185186

>From 887bc7cb47ca042728073e4d39c20e30c5bcb59c Mon Sep 17 00:00:00 2001
From: tudinhh <anhtu.dinh1202 at gmail.com>
Date: Sat, 7 Mar 2026 22:25:12 +0100
Subject: [PATCH] [SLP] Fix misvectorization when converting commutative to
 non-commutative opcodes

The BinOpSameOpcodeHelper::getOperand function was incorrectly preserving
the original operand order when converting a commutative operation
(like Or/And/Mul) with a constant on the left-hand side into a
non-commutative operation (like Shl/Sub).

When the original constant was at position 0 (Pos == 0), the helper
returned {Constant, Variable}. For non-commutative target opcodes, this
resulted in mathematically incorrect IR, such as 'shl 0, %x' instead of
'%x << 0'.

This patch widens the existing fix (which previously only handled Sub) to
any non-commutative instruction using Instruction::isCommutative. This
ensures that for any directional operation, the variable is forced to
the left-hand side and the constant to the right-hand side.

Fixes #185186
---
 .../Transforms/Vectorize/SLPVectorizer.cpp    | 10 ++---
 .../X86/lhs-constant-non-cummutative.ll       | 39 +++++++++++++++++++
 2 files changed, 43 insertions(+), 6 deletions(-)
 create mode 100644 llvm/test/Transforms/SLPVectorizer/X86/lhs-constant-non-cummutative.ll

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 631af61e2cfba..df7885e31741e 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -1132,13 +1132,11 @@ class BinOpSameOpcodeHelper {
         break;
       }
       Value *LHS = I->getOperand(1 - Pos);
-      // constant + x cannot be -constant - x
-      // instead, it should be x - -constant
-      if (Pos == 1 ||
-          ((FromOpcode == Instruction::Add || FromOpcode == Instruction::Or ||
-            FromOpcode == Instruction::Xor) &&
-           ToOpcode == Instruction::Sub))
+      // If the target opcode is non-commutative (e.g., shl, sub),
+      // force the variable to the left and the constant to the right.
+      if (Pos == 1 || !Instruction::isCommutative(ToOpcode))
         return SmallVector<Value *>({LHS, RHS});
+
       return SmallVector<Value *>({RHS, LHS});
     }
   };
diff --git a/llvm/test/Transforms/SLPVectorizer/X86/lhs-constant-non-cummutative.ll b/llvm/test/Transforms/SLPVectorizer/X86/lhs-constant-non-cummutative.ll
new file mode 100644
index 0000000000000..5a33d5f5bac5b
--- /dev/null
+++ b/llvm/test/Transforms/SLPVectorizer/X86/lhs-constant-non-cummutative.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -S --passes=slp-vectorizer -slp-threshold=-50 -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
+
+define void @or_lhs_to_shl(ptr %p, ptr %s) {
+; CHECK-LABEL: define void @or_lhs_to_shl(
+; CHECK-SAME: ptr [[P:%.*]], ptr [[S:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[TMP0:%.*]] = load <4 x i16>, ptr [[P]], align 2
+; CHECK-NEXT:    [[TMP1:%.*]] = shl <4 x i16> [[TMP0]], <i16 1, i16 2, i16 3, i16 0>
+; CHECK-NEXT:    store <4 x i16> [[TMP1]], ptr [[S]], align 2
+; CHECK-NEXT:    ret void
+;
+entry:
+  %p1 = getelementptr i16, ptr %p, i64 1
+  %p2 = getelementptr i16, ptr %p, i64 2
+  %p3 = getelementptr i16, ptr %p, i64 3
+
+  %l0 = load i16, ptr %p, align 2
+  %l1 = load i16, ptr %p1, align 2
+  %l2 = load i16, ptr %p2, align 2
+  %l3 = load i16, ptr %p3, align 2
+
+  ; 3 shl instructions force the vectorizer to choose shl as the target
+  %op0 = shl i16 %l0, 1
+  %op1 = shl i16 %l1, 2
+  %op2 = shl i16 %l2, 3
+  %op3 = or i16 0, %l3 ; The buggy instruction
+
+  %s1 = getelementptr i16, ptr %s, i64 1
+  %s2 = getelementptr i16, ptr %s, i64 2
+  %s3 = getelementptr i16, ptr %s, i64 3
+
+  store i16 %op0, ptr %s, align 2
+  store i16 %op1, ptr %s1, align 2
+  store i16 %op2, ptr %s2, align 2
+  store i16 %op3, ptr %s3, align 2
+
+  ret void
+}