[LAA] Fix type mismatch in getStartAndEndForAccess.#183116
Merged
[LAA] Fix type mismatch in getStartAndEndForAccess.#183116
Conversation
Member
|
@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. |
fhahn
reviewed
Feb 24, 2026
| @@ -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 | |||
Contributor
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
Contributor
Author
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. |
Contributor
Author
|
ping |
fhahn
reviewed
Mar 3, 2026
Comment on lines
+257
to
+258
| DerefBytesSCEV = SE.getNoopOrAnyExtend(DerefBytesSCEV, CommonTy); | ||
| DerefRKSCEV = SE.getNoopOrAnyExtend(DerefRKSCEV, CommonTy); |
Contributor
There was a problem hiding this comment.
Why any extend? Both quantities should be positive, so better always use Zext?
fhahn
reviewed
Mar 4, 2026
| ret void | ||
| } | ||
|
|
||
| define void @test_assumed_bounds_type_mismatch(ptr noalias %array, ptr readonly %pred, i32 %n) nosync nofree { |
Contributor
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
Contributor
Author
There was a problem hiding this comment.
yes the store is redundant in this case, needs to be removed.
Contributor
Author
There was a problem hiding this comment.
removing the store instruction is causing faulting load, instead of being vectorized.
fhahn
approved these changes
Mar 11, 2026
llvm/test/Transforms/LoopVectorize/early_exit_store_legality.ll
Outdated
Show resolved
Hide resolved
Contributor
Author
The test will not be vectorized as it is potentially an early exit loop due to and removing the store instruction causes faulting load. |
Contributor
It's fine as is. I also went ahead and clarified the PR title, hope that's OK with you |
Contributor
Author
|
Thanks, no issues with the PR title. |
Contributor
sure |
Contributor
|
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. |
Contributor
Author
Sure, will change. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.