[LAA] Fix type mismatch in getStartAndEndForAccess.#183116
[LAA] Fix type mismatch in getStartAndEndForAccess.#183116
Conversation
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Kshitij Paranjape (kshitijvp) Changes
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/LoopAccessAnalysis.cpp#L253 Running This is caused by a type mismatch between Full diff: https://github.com/llvm/llvm-project/pull/183116.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 057636050db25..67530ac4ba31d 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -250,8 +250,13 @@ static bool evaluatePtrAddRecAtMaxBTCWillNotWrap(
return true;
});
if (DerefRK) {
- DerefBytesSCEV =
- SE.getUMaxExpr(DerefBytesSCEV, SE.getSCEV(DerefRK.IRArgValue));
+ const SCEV *DerefRKSCEV = SE.getSCEV(DerefRK.IRArgValue);
+ // Ensure both operands have the same type
+ Type *CommonTy =
+ SE.getWiderType(DerefBytesSCEV->getType(), DerefRKSCEV->getType());
+ DerefBytesSCEV = SE.getNoopOrAnyExtend(DerefBytesSCEV, CommonTy);
+ DerefRKSCEV = SE.getNoopOrAnyExtend(DerefRKSCEV, CommonTy);
+ DerefBytesSCEV = SE.getUMaxExpr(DerefBytesSCEV, DerefRKSCEV);
}
if (DerefBytesSCEV->isZero())
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.ll b/llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.ll
new file mode 100644
index 0000000000000..0a28775ca290e
--- /dev/null
+++ b/llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.ll
@@ -0,0 +1,34 @@
+; RUN: opt -S -p loop-vectorize -debug-only=loop-vectorize -force-vector-width=4 -disable-output 2>&1 < %s | FileCheck %s
+; REQUIRES: asserts
+
+; CHECK-LABEL: LV: Checking a loop in 'test_assumed_bounds_type_mismatch'
+; CHECK: LV: Not vectorizing: Cannot vectorize uncountable loop.
+
+define void @test_assumed_bounds_type_mismatch(ptr noalias %array, ptr readonly %pred, i32 %n) nosync nofree {
+entry:
+ %n_bytes = mul nuw nsw i32 %n, 2
+ call void @llvm.assume(i1 true) [ "dereferenceable"(ptr %pred, i32 %n_bytes) ]
+ %tc = sext i32 %n to i64
+ br label %for.body
+
+for.body:
+ %iv = phi i64 [ 0, %entry ], [ %iv.next, %for.inc ]
+ %st.addr = getelementptr inbounds i16, ptr %array, i64 %iv
+ %data = load i16, ptr %st.addr, align 2
+ %inc = add nsw i16 %data, 1
+ store i16 %inc, ptr %st.addr, align 2
+ %ee.addr = getelementptr inbounds i16, ptr %pred, i64 %iv
+ %ee.val = load i16, ptr %ee.addr, align 2
+ %ee.cond = icmp sgt i16 %ee.val, 500
+ br i1 %ee.cond, label %exit, label %for.inc
+
+for.inc:
+ %iv.next = add nuw nsw i64 %iv, 1
+ %counted.cond = icmp eq i64 %iv.next, %tc
+ br i1 %counted.cond, label %exit, label %for.body
+
+exit:
+ ret void
+}
+
+declare void @llvm.assume(i1)
\ No newline at end of file
|
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
| @@ -0,0 +1,34 @@ | |||
| ; RUN: opt -S -p loop-vectorize -debug-only=loop-vectorize -force-vector-width=4 -disable-output 2>&1 < %s | FileCheck %s | |||
There was a problem hiding this comment.
you'd need to use -passes=print<aceess-info> for tests in this directory.
would be good to add to other existing early exit tests
There was a problem hiding this comment.
I have added to the existing one
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
|
ping |
| DerefBytesSCEV = SE.getNoopOrAnyExtend(DerefBytesSCEV, CommonTy); | ||
| DerefRKSCEV = SE.getNoopOrAnyExtend(DerefRKSCEV, CommonTy); |
There was a problem hiding this comment.
Why any extend? Both quantities should be positive, so better always use Zext?
| ret void | ||
| } | ||
|
|
||
| define void @test_assumed_bounds_type_mismatch(ptr noalias %array, ptr readonly %pred, i32 %n) nosync nofree { |
There was a problem hiding this comment.
is there a reason we need a loop with a store? If not, would be good to remove the store and add to llvm/test/Transforms/LoopVectorize/dereferenceable-info-from-assumption-variable-size.ll. It should get vectorized with the crash fixed
There was a problem hiding this comment.
yes the store is redundant in this case, needs to be removed.
There was a problem hiding this comment.
removing the store instruction is causing faulting load, instead of being vectorized.
The test will not be vectorized as it is potentially an early exit loop due to and removing the store instruction causes faulting load. |
It's fine as is. I also went ahead and clarified the PR title, hope that's OK with you |
|
Thanks, no issues with the PR title. |
sure |
|
I think you do not need to put complete backtrace in the commit message, we do not do it in practice, describing the issue and solution is enough. |
Sure, will change. |
`SE.getUMaxExpr` causes assertion failure due to type mismatch here: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/LoopAccessAnalysis.cpp#L253 Running `opt -S -p loop-vectorize -debug-only=loop-vectorize llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.ll ` without the changes made in LoopAccessAnalysis.cpp causes assertion failure. Attaching the stack dump for reference: ``` LV: Checking a loop in 'loop_contains_store_assumed_bounds' from input.ll LV: Loop hints: force=? width=4 interleave=0 LV: Found a loop: for.body LV: Found an induction variable. opt: /home/kshitij/llvm-project/llvm/lib/Analysis/ScalarEvolution.cpp:3918: const llvm::SCEV* llvm::ScalarEvolution::getMinMaxExpr(llvm::SCEVTypes, llvm::SmallVectorImpl<const llvm::SCEV*>&): Assertion `getEffectiveSCEVType(Ops[i]->getType()) == ETy && "Operand types don't match!"' failed. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace and instructions to reproduce the bug. Stack dump: 0. Program arguments: opt -S -passes=loop-vectorize -debug-only=loop-vectorize -force-vector-width=4 -disable-output input.ll 1. Running pass "function(loop-vectorize<no-interleave-forced-only;no-vectorize-forced-only;>)" on module "input.ll" 2. Running pass "loop-vectorize<no-interleave-forced-only;no-vectorize-forced-only;>" on function "loop_contains_store_assumed_bounds" #0 0x000058ee97c5e652 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/usr/local/bin/opt+0x4f44652) llvm#1 0x000058ee97c5af0f llvm::sys::RunSignalHandlers() (/usr/local/bin/opt+0x4f40f0f) llvm#2 0x000058ee97c5b05c SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0 llvm#3 0x00007c49d4c45330 (/lib/x86_64-linux-gnu/libc.so.6+0x45330) llvm#4 0x00007c49d4c9eb2c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76 llvm#5 0x00007c49d4c9eb2c __pthread_kill_internal ./nptl/pthread_kill.c:78:10 llvm#6 0x00007c49d4c9eb2c pthread_kill ./nptl/pthread_kill.c:89:10 llvm#7 0x00007c49d4c4527e raise ./signal/../sysdeps/posix/raise.c:27:6 llvm#8 0x00007c49d4c288ff abort ./stdlib/abort.c:81:7 llvm#9 0x00007c49d4c2881b _nl_load_domain ./intl/loadmsgcat.c:1177:9 llvm#10 0x00007c49d4c3b517 (/lib/x86_64-linux-gnu/libc.so.6+0x3b517) llvm#11 0x000058ee98003fdb llvm::ScalarEvolution::getMinMaxExpr(llvm::SCEVTypes, llvm::SmallVectorImpl<llvm::SCEV const*>&) (/usr/local/bin/opt+0x52e9fdb) llvm#12 0x000058ee98004507 llvm::ScalarEvolution::getUMaxExpr(llvm::SCEV const*, llvm::SCEV const*) (/usr/local/bin/opt+0x52ea507) llvm#13 0x000058ee980dc728 llvm::getStartAndEndForAccess(llvm::Loop const*, llvm::SCEV const*, llvm::Type*, llvm::SCEV const*, llvm::SCEV const*, llvm::ScalarEvolution*, llvm::DenseMap<std::pair<llvm::SCEV const*, llvm::Type*>, std::pair<llvm::SCEV const*, llvm::SCEV const*>, llvm::DenseMapInfo<std::pair<llvm::SCEV const*, llvm::Type*>, void>, llvm::detail::DenseMapPair<std::pair<llvm::SCEV const*, llvm::Type*>, std::pair<llvm::SCEV const*, llvm::SCEV const*>>>*, llvm::DominatorTree*, llvm::AssumptionCache*, std::optional<llvm::ScalarEvolution::LoopGuards>&) (/usr/local/bin/opt+0x53c2728) llvm#14 0x000058ee9814008b llvm::isDereferenceableAndAlignedInLoop(llvm::LoadInst*, llvm::Loop*, llvm::ScalarEvolution&, llvm::DominatorTree&, llvm::AssumptionCache*, llvm::SmallVectorImpl<llvm::SCEVPredicate const*>*) (/usr/local/bin/opt+0x542608b) llvm#15 0x000058ee9a0fa1ca llvm::LoopVectorizationLegality::canUncountableExitConditionLoadBeMoved(llvm::BasicBlock*) (/usr/local/bin/opt+0x73e01ca) llvm#16 0x000058ee9a0faee0 llvm::LoopVectorizationLegality::isVectorizableEarlyExitLoop() (/usr/local/bin/opt+0x73e0ee0) llvm#17 0x000058ee9a104678 llvm::LoopVectorizationLegality::canVectorize(bool) (/usr/local/bin/opt+0x73ea678) llvm#18 0x000058ee9a08c953 llvm::LoopVectorizePass::processLoop(llvm::Loop*) (/usr/local/bin/opt+0x7372953) llvm#19 0x000058ee9a090e21 llvm::LoopVectorizePass::runImpl(llvm::Function&) (/usr/local/bin/opt+0x7376e21) llvm#20 0x000058ee9a0914e0 llvm::LoopVectorizePass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/bin/opt+0x73774e0) llvm#21 0x000058ee99e419a5 llvm::detail::PassModel<llvm::Function, llvm::LoopVectorizePass, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0 llvm#22 0x000058ee97f18905 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/bin/opt+0x51fe905) llvm#23 0x000058ee995d70d5 llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) AMDGPUTargetMachine.cpp:0:0 llvm#24 0x000058ee97f17051 llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/bin/opt+0x51fd051) llvm#25 0x000058ee995d7775 llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) AMDGPUTargetMachine.cpp:0:0 llvm#26 0x000058ee97f1783d llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/bin/opt+0x51fd83d) llvm#27 0x000058ee9c153909 llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool, bool) (/usr/local/bin/opt+0x9439909) llvm#28 0x000058ee97c3f380 optMain (/usr/local/bin/opt+0x4f25380) llvm#29 0x00007c49d4c2a1ca __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3 llvm#30 0x00007c49d4c2a28b call_init ./csu/../csu/libc-start.c:128:20 llvm#31 0x00007c49d4c2a28b __libc_start_main ./csu/../csu/libc-start.c:347:5 llvm#32 0x000058ee97c309a5 _start (/usr/local/bin/opt+0x4f169a5) ``` This is caused by a type mismatch between `SE.getSCEV(DerefRK.IRArgValue)` and `DerefBytesSCEV`. Fixing this by extending them to the wider type.
SE.getUMaxExprcauses assertion failure due to type mismatch here:https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/LoopAccessAnalysis.cpp#L253
Running
opt -S -p loop-vectorize -debug-only=loop-vectorize llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.llwithout the changes made in LoopAccessAnalysis.cpp causes assertion failure.This is caused by a type mismatch between
SE.getSCEV(DerefRK.IRArgValue)andDerefBytesSCEV.Fixing this by extending them to the wider type.