Closed
Description
I'm debugging an issue in wamrc, the ahead-of-time WASM compiler for the Web Assembly Micro Runtime (WAMR). For certain inputs, the compiler crashes with
assertion __comp(*(__first + __a), *(__first + __b)) failed: Your comparator is not a valid strict-weak ordering
Debugging this reveals a crash in LLVM:
* thread #1, name = 'wamrc', stop reason = SIGABRT
* frame #0: 0x00007f2cc2f20981 libc.so.6`raise + 161
frame #1: 0x00007f2cc2f21df7 libc.so.6`abort + 247
frame #2: 0x0000561f5ec238ad wamrc`std::__u::__libcpp_verbose_abort(char const*, ...) + 173
frame #3: 0x0000561f5db1860e wamrc`void std::__u::__check_strict_weak_ordering_sorted<llvm::StoreInst**, llvm::function_ref<bool (llvm::StoreInst*, llvm::StoreInst*)>>(llvm::StoreInst**, llvm::StoreInst**, llvm::function_ref<bool (llvm::StoreInst*, llvm::StoreInst*)>&) + 574
frame #4: 0x0000561f5da9dc9a wamrc`llvm::SLPVectorizerPass::vectorizeStoreChains(llvm::slpvectorizer::BoUpSLP&) + 826
frame #5: 0x0000561f5da9ca70 wamrc`llvm::SLPVectorizerPass::runImpl(llvm::Function&, llvm::ScalarEvolution*, llvm::TargetTransformInfo*, llvm::TargetLibraryInfo*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::AssumptionCache*, llvm::DemandedBits*, llvm::OptimizationRemarkEmitter*) + 1824
frame #6: 0x0000561f5da9c199 wamrc`llvm::SLPVectorizerPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) + 1385
frame #7: 0x0000561f5b3bd9e2 wamrc`llvm::detail::PassModel<llvm::Function, llvm::SLPVectorizerPass, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) + 18
frame #8: 0x0000561f5e8aa137 wamrc`llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) + 615
frame #9: 0x0000561f5b3beee2 wamrc`llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) + 18
frame #10: 0x0000561f5e8ad2f8 wamrc`llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) + 680
frame #11: 0x0000561f5b3bf142 wamrc`llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) + 18
frame #12: 0x0000561f5e8a9197 wamrc`llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) + 743
frame #13: 0x0000561f5b3b9aa4 wamrc`aot_apply_llvm_new_pass_manager + 6708
frame #14: 0x0000561f5b37f498 wamrc`aot_compile_wasm + 280
frame #15: 0x0000561f5b3537d2 wamrc`main + 4290
frame #16: 0x00007f2cc2f0c3d4 libc.so.6`__libc_start_main + 244
frame #17: 0x0000561f5b3526aa wamrc`_start + 42
The culprit appears to be in StoreSorter, which contains some code that is pretty fishy for a comparison function:
if (isa<UndefValue>(V->getValueOperand()) ||
isa<UndefValue>(V2->getValueOperand()))
return false;If both StoreInsts are have UndefValue operands, then the function does not impose a strict ordering on them, which is what the assertion is checking for.
Would a fix like the following work?
diff --git a/llvm-project/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm-project/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
--- a/llvm-project/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -21671,28 +21671,21 @@ bool SLPVectorizerPass::vectorizeStoreCh
// compatible (have the same opcode, same parent), otherwise it is
// definitely not profitable to try to vectorize them.
auto &&StoreSorter = [this](StoreInst *V, StoreInst *V2) {
- if (V->getValueOperand()->getType()->getTypeID() <
- V2->getValueOperand()->getType()->getTypeID())
- return true;
- if (V->getValueOperand()->getType()->getTypeID() >
- V2->getValueOperand()->getType()->getTypeID())
- return false;
- if (V->getPointerOperandType()->getTypeID() <
- V2->getPointerOperandType()->getTypeID())
- return true;
- if (V->getPointerOperandType()->getTypeID() >
- V2->getPointerOperandType()->getTypeID())
- return false;
- if (V->getValueOperand()->getType()->getScalarSizeInBits() <
- V2->getValueOperand()->getType()->getScalarSizeInBits())
- return true;
- if (V->getValueOperand()->getType()->getScalarSizeInBits() >
- V2->getValueOperand()->getType()->getScalarSizeInBits())
- return false;
- // UndefValues are compatible with all other values.
- if (isa<UndefValue>(V->getValueOperand()) ||
- isa<UndefValue>(V2->getValueOperand()))
- return false;
+ if (auto T1 = V->getValueOperand()->getType()->getTypeID(),
+ T2 = V2->getValueOperand()->getType()->getTypeID();
+ T1 != T2) {
+ return T1 > T2;
+ }
+ if (auto T1 = V->getPointerOperandType()->getTypeID(),
+ T2 = V2->getPointerOperandType()->getTypeID();
+ T1 != T2) {
+ return T1 < T2;
+ }
+ if (auto S1 = V->getValueOperand()->getType()->getScalarSizeInBits(),
+ S2 = V2->getValueOperand()->getType()->getScalarSizeInBits();
+ S1 != S2) {
+ return S1 < S2;
+ }
if (auto *I1 = dyn_cast<Instruction>(V->getValueOperand()))
if (auto *I2 = dyn_cast<Instruction>(V2->getValueOperand())) {
DomTreeNodeBase<llvm::BasicBlock> *NodeI1 =
@@ -21706,14 +21699,8 @@ bool SLPVectorizerPass::vectorizeStoreCh
"Different nodes should have different DFS numbers");
if (NodeI1 != NodeI2)
return NodeI1->getDFSNumIn() < NodeI2->getDFSNumIn();
- InstructionsState S = getSameOpcode({I1, I2}, *TLI);
- if (S.getOpcode())
- return false;
return I1->getOpcode() < I2->getOpcode();
}
- if (isa<Constant>(V->getValueOperand()) &&
- isa<Constant>(V2->getValueOperand()))
- return false;
return V->getValueOperand()->getValueID() <
V2->getValueOperand()->getValueID();
};