Revert "[SLP]Check if the copyable element is a sub instruciton with abs in isCommutable"

[Prabhuk]

This reverts commit ddf5bb0.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Prabhu Rajasekaran (Prabhuk)

Changes

This reverts commit ddf5bb0.

---

Full diff: https://github.com/llvm/llvm-project/pull/168148.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+4-7)
• (removed) llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll (-29)

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp index 938eacde7548d..e61eb0fcfe492 100644 --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp @@ -537,8 +537,7 @@ static bool isSplat(ArrayRef<Value *> VL) { /// \param I The instruction to check for commutativity /// \param ValWithUses The value whose uses are analyzed for special /// patterns -static bool isCommutative(Instruction *I, Value *ValWithUses, - bool IsCopyable = false) { +static bool isCommutative(Instruction *I, Value *ValWithUses) { if (auto *Cmp = dyn_cast<CmpInst>(I)) return Cmp->isCommutative(); if (auto *BO = dyn_cast<BinaryOperator>(I)) @@ -547,7 +546,7 @@ static bool isCommutative(Instruction *I, Value *ValWithUses, !ValWithUses->hasNUsesOrMore(UsesLimit) && all_of( ValWithUses->uses(), - [&](const Use &U) { + [](const Use &U) { // Commutative, if icmp eq/ne sub, 0 CmpPredicate Pred; if (match(U.getUser(), @@ -556,11 +555,10 @@ static bool isCommutative(Instruction *I, Value *ValWithUses, return true; // Commutative, if abs(sub nsw, true) or abs(sub, false). ConstantInt *Flag; - auto *I = dyn_cast<BinaryOperator>(U.get()); return match(U.getUser(), m_Intrinsic<Intrinsic::abs>( m_Specific(U.get()), m_ConstantInt(Flag))) && - ((!IsCopyable && I && !I->hasNoSignedWrap()) || + (!cast<Instruction>(U.get())->hasNoSignedWrap() || Flag->isOne()); })) || (BO->getOpcode() == Instruction::FSub && @@ -3166,8 +3164,7 @@ class BoUpSLP { bool IsInverseOperation = false; if (S.isCopyableElement(VL[Lane])) { // The value is a copyable element. - IsInverseOperation = - !isCommutative(MainOp, VL[Lane], /*IsCopyable=*/true); + IsInverseOperation = !isCommutative(MainOp, VL[Lane]); } else { assert(I && "Expected instruction"); auto [SelectedOp, Ops] = convertTo(I, S); diff --git a/llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll b/llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll deleted file mode 100644 index d90873f1895e4..0000000000000 --- a/llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll +++ /dev/null @@ -1,29 +0,0 @@ -; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6 -; RUN: opt -passes=slp-vectorizer -S -slp-threshold=-100 -mtriple=arm64-apple-macosx15.0.0 < %s | FileCheck %s - -define i1 @test(i32 %shr.i.i90, i32 %x) { -; CHECK-LABEL: define i1 @test( -; CHECK-SAME: i32 [[SHR_I_I90:%.*]], i32 [[X:%.*]]) { -; CHECK-NEXT: [[ENTRY:.*:]] -; CHECK-NEXT: [[SUB32_I_I:%.*]] = sub i32 [[X]], 2 -; CHECK-NEXT: [[TMP0:%.*]] = insertelement <2 x i32> poison, i32 [[SHR_I_I90]], i32 1 -; CHECK-NEXT: [[TMP1:%.*]] = insertelement <2 x i32> [[TMP0]], i32 [[SUB32_I_I]], i32 0 -; CHECK-NEXT: [[TMP2:%.*]] = call <2 x i32> @llvm.abs.v2i32(<2 x i32> [[TMP1]], i1 true) -; CHECK-NEXT: [[TMP3:%.*]] = zext <2 x i32> [[TMP2]] to <2 x i64> -; CHECK-NEXT: [[TMP4:%.*]] = icmp ugt <2 x i64> [[TMP3]], <i64 100, i64 300> -; CHECK-NEXT: [[TMP5:%.*]] = extractelement <2 x i1> [[TMP4]], i32 0 -; CHECK-NEXT: ret i1 [[TMP5]] -; -entry: - %cond.i.i = tail call i32 @llvm.abs.i32(i32 %shr.i.i90, i1 true) - %conv.i.i91 = zext i32 %cond.i.i to i64 - %sub32.i.i = sub i32 %x, 2 - %cond41.i.i = tail call i32 @llvm.abs.i32(i32 %sub32.i.i, i1 true) - %conv42.i.i = zext i32 %cond41.i.i to i64 - %cmp.not.i.2.i.i = icmp ugt i64 %conv.i.i91, 300 - %cmp.not.i.3.i.i = icmp ugt i64 %conv42.i.i, 100 - ret i1 %cmp.not.i.3.i.i -} - -; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none) -declare i32 @llvm.abs.i32(i32, i1 immarg) #0 

[Prabhuk]

@alexey-bataev your commit to main broke our builders in Google. Since I could not find a PR for this change to discuss this I am directly reverting this patch. Please let me know if we can help in anyway to help you reland this but a PR would help.

Exit Code: 1 Command Output (stdout): -- # RUN: at line 2 /b/s/w/ir/x/w/llvm_build/bin/opt -passes=slp-vectorizer -S -slp-threshold=-100 -mtriple=arm64-apple-macosx15.0.0 < /b/s/w/ir/x/w/llvm-llvm-project/llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll | /b/s/w/ir/x/w/llvm_build/bin/FileCheck /b/s/w/ir/x/w/llvm-llvm-project/llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll # executed command: /b/s/w/ir/x/w/llvm_build/bin/opt -passes=slp-vectorizer -S -slp-threshold=-100 -mtriple=arm64-apple-macosx15.0.0 # executed command: /b/s/w/ir/x/w/llvm_build/bin/FileCheck /b/s/w/ir/x/w/llvm-llvm-project/llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll # .---command stderr------------ # | /b/s/w/ir/x/w/llvm-llvm-project/llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll:8:15: error: CHECK-NEXT: expected string not found in input # | ; CHECK-NEXT: [[SUB32_I_I:%.*]] = sub i32 [[X]], 2 # | ^ # | <stdin>:7:7: note: scanning from here # | entry: # | ^ # | <stdin>:7:7: note: with "X" equal to "%x" # | entry: # | ^ # | <stdin>:10:2: note: possible intended match here # | %2 = sub <2 x i32> %1, <i32 2, i32 0> # | ^ # | # | Input file: <stdin> # | Check file: /b/s/w/ir/x/w/llvm-llvm-project/llvm/test/Transforms/SLPVectorizer/AArch64/non-inst-abs-sub-copyable-value.ll # | # | -dump-input=help explains the following input dump. # | # | Input was: # | <<<<<< # | 1: ; ModuleID = '<stdin>' # | 2: source_filename = "<stdin>" # | 3: target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-n32:64-S128-Fn32" # | 4: target triple = "arm64-apple-macosx15.0.0" # | 5: # | 6: define i1 @test(i32 %shr.i.i90, i32 %x) { # | 7: entry: # | next:8'0 X error: no match found # | next:8'1 with "X" equal to "%x" # | 8: %0 = insertelement <2 x i32> poison, i32 %x, i32 0 # | next:8'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | 9: %1 = insertelement <2 x i32> %0, i32 %shr.i.i90, i32 1 # | next:8'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | 10: %2 = sub <2 x i32> %1, <i32 2, i32 0> # | next:8'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | next:8'2 ? possible intended match # | 11: %3 = call <2 x i32> @llvm.abs.v2i32(<2 x i32> %2, i1 true) # | next:8'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | 12: %4 = zext <2 x i32> %3 to <2 x i64> # | next:8'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | 13: %5 = icmp ugt <2 x i64> %4, <i64 100, i64 300> # | next:8'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | 14: %6 = extractelement <2 x i1> %5, i32 0 # | next:8'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | 15: ret i1 %6 # | next:8'0 ~~~~~~~~~~~ # | . # | . # | . # | >>>>>> # `----------------------------- # error: command failed with exit status: 1 

[alexey-bataev]

Already reverted

[Prabhuk]

@alexey-bataev - thank you!