Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SLPVectorizer] Assertion `idx < size()' failed. #49074

Closed
llvmbot opened this issue Mar 26, 2021 · 11 comments
Closed

[SLPVectorizer] Assertion `idx < size()' failed. #49074

llvmbot opened this issue Mar 26, 2021 · 11 comments
Labels
bugzilla Issues migrated from bugzilla

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 26, 2021

Bugzilla Link 49730
Resolution FIXED
Resolved on Mar 31, 2021 07:12
Version trunk
OS Windows NT
Reporter LLVM Bugzilla Contributor
CC @alexey-bataev,@annamthomas,@dtemirbulatov,@fhahn,@RKSimon,@rotateright
Fixed by commit(s) da381cf

Extended Description

To reproduce: run opt -S -passes=slp-vectorizer on the following test:

; ModuleID = './bugpoint-reduced-simplified.bc'
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
target triple = "x86_64-unknown-linux-gnu"

define void @​bar() {
bb:
%tmp = call i32 @​llvm.smin.i32(i32 undef, i32 2)
%tmp1 = sub nsw i32 undef, %tmp
%tmp2 = call i32 @​llvm.umin.i32(i32 undef, i32 %tmp1)
%tmp3 = call i32 @​llvm.smin.i32(i32 undef, i32 2)
%tmp4 = sub nsw i32 undef, %tmp3
%tmp5 = call i32 @​llvm.umin.i32(i32 %tmp2, i32 %tmp4)
%tmp6 = call i32 @​llvm.smin.i32(i32 undef, i32 1)
%tmp7 = sub nuw nsw i32 undef, %tmp6
%tmp8 = call i32 @​llvm.umin.i32(i32 %tmp5, i32 %tmp7)
%tmp9 = call i32 @​llvm.smin.i32(i32 undef, i32 1)
%tmp10 = sub nsw i32 undef, %tmp9
%tmp11 = call i32 @​llvm.umin.i32(i32 %tmp8, i32 %tmp10)
%tmp12 = sub nsw i32 undef, undef
%tmp13 = call i32 @​llvm.umin.i32(i32 %tmp11, i32 %tmp12)
%tmp14 = call i32 @​llvm.umin.i32(i32 %tmp13, i32 93)
ret void
}

; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
declare i32 @​llvm.smin.i32(i32, i32)

; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
declare i32 @​llvm.umin.i32(i32, i32)

The failure is:
opt: /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallVector.h:281: const T& llvm::SmallVectorTemplateCommon<T, >::operator[](llvm::SmallVectorTemplateCommon<T, >::size_type) const [with T = llvm::SmallVector<llvm::Value*, 16>; = void; llvm::SmallVectorTemplateCommon<T, >::const_reference = const llvm::SmallVector<llvm::Value*, 16>&; llvm::SmallVectorTemplateCommon<T, >::size_type = long unsigned int]: Assertion `idx < size()' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0. Program arguments: /home/mkazantsev/work/orca_install/RA/bin/opt -S -passes=slp-vectorizer ./simple.ll
#​0 0x00007f9b9bf6ed4c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /localhome/mkazantsev/work/orca/orca/llvm/lib/Support/Unix/Signals.inc:569:0
#​1 0x00007f9b9bf6cb64 llvm::sys::RunSignalHandlers() /localhome/mkazantsev/work/orca/orca/llvm/lib/Support/Signals.cpp:71:0
#​2 0x00007f9b9bf6d593 SignalHandler(int) /localhome/mkazantsev/work/orca/orca/llvm/lib/Support/Unix/Signals.inc:397:0
#​3 0x00007f9b9b494630 __restore_rt sigaction.c:0:0
#​4 0x00007f9b9a83c3d7 raise (/lib64/libc.so.6+0x363d7)
#​5 0x00007f9b9a83dac8 abort (/lib64/libc.so.6+0x37ac8)
#​6 0x00007f9b9a8351a6 __assert_fail_base (/lib64/libc.so.6+0x2f1a6)
#​7 0x00007f9b9a835252 (/lib64/libc.so.6+0x2f252)
#​8 0x00007f9b9d2af584 llvm::isa_impl_cl<llvm::SelectInst, llvm::Value const*>::doit(llvm::Value const*) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/Support/Casting.h:104:0
#​9 0x00007f9b9d2af584 llvm::isa_impl_wrap<llvm::SelectInst, llvm::Value const*, llvm::Value const*>::doit(llvm::Value const* const&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/Support/Casting.h:131:0
#​10 0x00007f9b9d2af584 llvm::isa_impl_wrap<llvm::SelectInst, llvm::Value* const, llvm::Value const*>::doit(llvm::Value* const&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/Support/Casting.h:123:0
#​11 0x00007f9b9d2af584 bool llvm::isa<llvm::SelectInst, llvm::Value*>(llvm::Value* const&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/Support/Casting.h:143:0
#​12 0x00007f9b9d2af584 llvm::cast_retty<llvm::SelectInst, llvm::Value*>::ret_type llvm::dyn_cast<llvm::SelectInst, llvm::Value>(llvm::Value*) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/Support/Casting.h:345:0
#​13 0x00007f9b9d2af584 (anonymous namespace)::HorizontalReduction::createOp(llvm::IRBuilder<llvm::ConstantFolder, llvm::IRBuilderDefaultInserter>&, llvm::RecurKind, llvm::Value*, llvm::Value*, llvm::Twine const&, llvm::SmallVector<llvm::SmallVector<llvm::Value*, 16u>, 2u> const&) /localhome/mkazantsev/work/orca/orca/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6691:0
#​14 0x00007f9b9d2dd85e tryToReduce /localhome/mkazantsev/work/orca/orca/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7158:0
#​15 0x00007f9b9d2dd85e tryToVectorizeHorReductionOrInstOperands(llvm::PHINode*, llvm::Instruction*, llvm::BasicBlock*, llvm::slpvectorizer::BoUpSLP&, llvm::TargetTransformInfo*, llvm::function_ref<bool (llvm::Instruction*, llvm::slpvectorizer::BoUpSLP&)>) (.constprop.1675) /localhome/mkazantsev/work/orca/orca/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7520:0
#​16 0x00007f9b9d2de35d llvm::SLPVectorizerPass::vectorizeRootInstruction(llvm::PHINode*, llvm::Value*, llvm::BasicBlock*, llvm::slpvectorizer::BoUpSLP&, llvm::TargetTransformInfo*) /localhome/mkazantsev/work/orca/orca/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7575:0
#​17 0x00007f9b9d2df29f llvm::SLPVectorizerPass::vectorizeChainsInBlock(llvm::BasicBlock*, llvm::slpvectorizer::BoUpSLP&) /localhome/mkazantsev/work/orca/orca/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7770:0
#​18 0x00007f9b9d2dfd4c llvm::SLPVectorizerPass::runImpl(llvm::Function&, llvm::ScalarEvolution*, llvm::TargetTransformInfo*, llvm::TargetLibraryInfo*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::AssumptionCache*, llvm::DemandedBits*, llvm::OptimizationRemarkEmitter*) (.part.1670) /localhome/mkazantsev/work/orca/orca/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6097:0
#​19 0x00007f9b9d2e0496 llvm::SLPVectorizerPass::run(llvm::Function&, llvm::AnalysisManagerllvm::Function&) /localhome/mkazantsev/work/orca/orca/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6035:0
#​20 0x00007f9b9e0251cd llvm::detail::PassModel<llvm::Function, llvm::SLPVectorizerPass, llvm::PreservedAnalyses, llvm::AnalysisManagerllvm::Function >::run(llvm::Function&, llvm::AnalysisManagerllvm::Function&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManagerInternal.h:86:0
#​21 0x00007f9b9c0f9659 llvm::SmallPtrSet<void*, 2u>::operator=(llvm::SmallPtrSet<void*, 2u>&&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallPtrSet.h:488:0
#​22 0x00007f9b9c0f9659 llvm::PreservedAnalyses::operator=(llvm::PreservedAnalyses&&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManager.h:155:0
#​23 0x00007f9b9c0f9659 llvm::PassManager<llvm::Function, llvm::AnalysisManagerllvm::Function >::run(llvm::Function&, llvm::AnalysisManagerllvm::Function&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManager.h:517:0
#​24 0x00007f9b9e020e3d llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManagerllvm::Function >, llvm::PreservedAnalyses, llvm::AnalysisManagerllvm::Function >::run(llvm::Function&, llvm::AnalysisManagerllvm::Function&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManagerInternal.h:86:0
#​25 0x00007f9b9c0f9157 llvm::SmallPtrSet<void*, 2u>::operator=(llvm::SmallPtrSet<void*, 2u>&&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallPtrSet.h:488:0
#​26 0x00007f9b9c0f9157 llvm::PreservedAnalyses::operator=(llvm::PreservedAnalyses&&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManager.h:155:0
#​27 0x00007f9b9c0f9157 llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManagerllvm::Module&) /localhome/mkazantsev/work/orca/orca/llvm/lib/IR/PassManager.cpp:117:0
#​28 0x000000000041e5dd llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManagerllvm::Module >::run(llvm::Module&, llvm::AnalysisManagerllvm::Module&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManagerInternal.h:86:0
#​29 0x00007f9b9c0f7a47 llvm::SmallPtrSet<void*, 2u>::operator=(llvm::SmallPtrSet<void*, 2u>&&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallPtrSet.h:488:0
#​30 0x00007f9b9c0f7a47 llvm::PreservedAnalyses::operator=(llvm::PreservedAnalyses&&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManager.h:155:0
#​31 0x00007f9b9c0f7a47 llvm::PassManager<llvm::Module, llvm::AnalysisManagerllvm::Module >::run(llvm::Module&, llvm::AnalysisManagerllvm::Module&) /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManager.h:517:0
#​32 0x0000000000427b92 llvm::SmallPtrSetImplBase::~SmallPtrSetImplBase() /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallPtrSet.h:82:0
#​33 0x0000000000427b92 llvm::SmallPtrSetImplllvm::AnalysisKey*::~SmallPtrSetImpl() /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallPtrSet.h:343:0
#​34 0x0000000000427b92 llvm::SmallPtrSet<llvm::AnalysisKey*, 2u>::~SmallPtrSet() /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallPtrSet.h:449:0
#​35 0x0000000000427b92 llvm::PreservedAnalyses::~PreservedAnalyses() /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/IR/PassManager.h:155:0
#​36 0x0000000000427b92 llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRefllvm::StringRef, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool) /localhome/mkazantsev/work/orca/orca/llvm/tools/opt/NewPMDriver.cpp:453:0
#​37 0x000000000041bfa2 llvm::SmallVectorTemplateCommon<llvm::StringRef, void>::end() /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallVector.h:255:0
#​38 0x000000000041bfa2 llvm::SmallVector<llvm::StringRef, 4u>::~SmallVector() /localhome/mkazantsev/work/orca/orca/llvm/include/llvm/ADT/SmallVector.h:1175:0
#​39 0x000000000041bfa2 main /localhome/mkazantsev/work/orca/orca/llvm/tools/opt/opt.cpp:814:0
#​40 0x00007f9b9a828555 __libc_start_main (/lib64/libc.so.6+0x22555)
#​41 0x000000000041c85e _start (/home/mkazantsev/work/orca_install/RA/bin/opt+0x41c85e)
Aborted (core dumped)

@fhahn
Copy link
Contributor

fhahn commented Mar 26, 2021

This looks similar to the issue fixed yesterday: https://reviews.llvm.org/rGf7ef26ef0b29a7c864209b78abf4445407a154b1

Does that commit fix the cash you are seeing?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 26, 2021

No, it's another one seen on freshest LLVM. Caused by

Revert of this patch helps:

commit 3c8473b

Author: Sanjay Patel spatel@rotateright.com
Date: Tue Mar 23 08:30:19 2021 -0400

[SLP] allow matching integer min/max intrinsics as reduction ops
As noted in D98152, we need to patch SLP to avoid regressions when
we start canonicalizing to integer min/max intrinsics.
Most of the real work to make this possible was in:
7202f47508
Differential Revision: https://reviews.llvm.org/D98981

@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 26, 2021

ReductionOps is empty and we crash trying to access its argument it seems.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 26, 2021

There are multiple places in code (e.g. initReductionOps, addReductionOps) that only recognize select instructions as subject of reduction, and they are not updated to handle the intrinsic form. I propose reverting the patch in question, and then we should thoroughly review all such places and understand what kind of update they need.

Sanjay, wdyt?

@rotateright
Copy link
Contributor

I thought I had fixed all of the select dependencies, but clearly I was wrong. :)
I'll look at the example here and see if I can patch it quickly, but we can revert and take a step back if it doesn't seem like a fast fix or there are multiple patches needed.

I am curious if the test was found by fuzzer or other means? We're not creating these intrinsics in other passes yet AFAIK.

@rotateright
Copy link
Contributor

I think there's an easy fix -- we need to create the intrinsics here in SLP to keep in sync with the pattern matching -- but this creates several regression test diffs, so I'm looking those over now.

Let me know if I should revert immediately, or I can post this with test updates on Phab shortly.

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 001cbe9fb0fe..e6c69cc9e81f 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -6588,18 +6588,22 @@ class HorizontalReduction {
return Builder.CreateBinaryIntrinsic(Intrinsic::minnum, LHS, RHS);

 case RecurKind::SMax: {
  •  return Builder.CreateBinaryIntrinsic(Intrinsic::smax, LHS, RHS);
     Value *Cmp = Builder.CreateICmpSGT(LHS, RHS, Name);
     return Builder.CreateSelect(Cmp, LHS, RHS, Name);
    
    }
    case RecurKind::SMin: {
  •  return Builder.CreateBinaryIntrinsic(Intrinsic::smin, LHS, RHS);
     Value *Cmp = Builder.CreateICmpSLT(LHS, RHS, Name);
     return Builder.CreateSelect(Cmp, LHS, RHS, Name);
    
    }
    case RecurKind::UMax: {
  •  return Builder.CreateBinaryIntrinsic(Intrinsic::umax, LHS, RHS);
     Value *Cmp = Builder.CreateICmpUGT(LHS, RHS, Name);
     return Builder.CreateSelect(Cmp, LHS, RHS, Name);
    
    }
    case RecurKind::UMin: {
  •  return Builder.CreateBinaryIntrinsic(Intrinsic::umin, LHS, RHS);
     Value *Cmp = Builder.CreateICmpULT(LHS, RHS, Name);
     return Builder.CreateSelect(Cmp, LHS, RHS, Name);
    
    }
    @@ -6615,10 +6619,11 @@ class HorizontalReduction {
    const ReductionOpsListType &ReductionOps) {
    Value *Op = createOp(Builder, RdxKind, LHS, RHS, Name);
    if (RecurrenceDescriptor::isIntMinMaxRecurrenceKind(RdxKind)) {
  •  if (auto *Sel = dyn_cast<SelectInst>(Op))
    
  •  if (auto *Sel = dyn_cast<SelectInst>(Op)) {
       propagateIRFlags(Sel->getCondition(), ReductionOps[0]);
    
  •  propagateIRFlags(Op, ReductionOps[1]);
    
  •  return Op;
    
  •    propagateIRFlags(Op, ReductionOps[1]);
    
  •    return Op;
    
  •  }
    
    }
    propagateIRFlags(Op, ReductionOps[0]);
    return Op;

@annamthomas
Copy link
Contributor

I think there's an easy fix -- we need to create the intrinsics here in SLP
to keep in sync with the pattern matching -- but this creates several
regression test diffs, so I'm looking those over now.

Let me know if I should revert immediately, or I can post this with test
updates on Phab shortly.
Hi Sanjay,

Could you please revert the patch and then update with the corrected patch for review? Thank you.

Also, answer to your previous question: yes, it was found by fuzzer, but it is blocking our nightly testing cycle and builds, because there are multiple failures.

@rotateright
Copy link
Contributor

I think there's an easy fix -- we need to create the intrinsics here in SLP
to keep in sync with the pattern matching -- but this creates several
regression test diffs, so I'm looking those over now.

Let me know if I should revert immediately, or I can post this with test
updates on Phab shortly.
Hi Sanjay,

Could you please revert the patch and then update with the corrected patch
for review? Thank you.

Reverted: https://reviews.llvm.org/rGa26312f9d4f2
Working on the update patch now.

Also, answer to your previous question: yes, it was found by fuzzer, but it
is blocking our nightly testing cycle and builds, because there are multiple
failures.

Great - thanks for the extra testing.

@rotateright
Copy link
Contributor

Updated patch posted for review:
https://reviews.llvm.org/D98981

The example from this bug report was added at:
https://reviews.llvm.org/rG6a7bcc9c8df8

I made that a positive test rather than just XFAIL with:
https://reviews.llvm.org/rG3f6e7d1550bc

@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 29, 2021

I am curious if the test was found by fuzzer or other means? We're not creating these intrinsics in other passes yet AFAIK.

Yes, it was found by our Java fuzzer.

@rotateright
Copy link
Contributor

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

4 participants