-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[LoopCacheAnalysis] Fix crash after #164798 #169486
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
Conversation
|
@llvm/pr-subscribers-llvm-analysis Author: Ryotaro Kasuga (kasuga-fj) ChangesFix the assertion failure after #164798. The issue is that the comparison Full diff: https://github.com/llvm/llvm-project/pull/169486.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/LoopCacheAnalysis.cpp b/llvm/lib/Analysis/LoopCacheAnalysis.cpp
index e0e2be8e35929..3bba2e8c0d8ad 100644
--- a/llvm/lib/Analysis/LoopCacheAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopCacheAnalysis.cpp
@@ -368,8 +368,16 @@ bool IndexedReference::tryDelinearizeFixedSize(
// the load/store instruction being analyzed. It is not needed for further
// analysis.
// TODO: Maybe this property should be enforced in delinearizeFixedSizeArray.
+#ifndef NDEBUG
assert(!Sizes.empty() && Subscripts.size() == Sizes.size() &&
- Sizes.back() == ElementSize && "Unexpected delinearization result");
+ "Inconsistent length of Sizes and Subscripts");
+ Type *WideTy =
+ SE.getWiderType(ElementSize->getType(), Sizes.back()->getType());
+ const SCEV *ElemSizeExt = SE.getNoopOrZeroExtend(ElementSize, WideTy);
+ const SCEV *LastSizeExt = SE.getNoopOrZeroExtend(Sizes.back(), WideTy);
+ assert(ElemSizeExt == LastSizeExt && "Unexpected last element of Sizes");
+#endif
+
Sizes.pop_back();
return true;
}
diff --git a/llvm/test/Analysis/LoopCacheAnalysis/crash-after-pr164798.ll b/llvm/test/Analysis/LoopCacheAnalysis/crash-after-pr164798.ll
new file mode 100644
index 0000000000000..e6b6d1753adb7
--- /dev/null
+++ b/llvm/test/Analysis/LoopCacheAnalysis/crash-after-pr164798.ll
@@ -0,0 +1,33 @@
+; RUN: opt < %s -passes='print<loop-cache-cost>' -disable-output
+
+; Ensure no crash happens after PR #164798
+
+target datalayout = "p21:32:16"
+
+define i16 @f() {
+entry:
+ br label %for.cond1.preheader
+
+for.cond1.preheader:
+ %i.02 = phi i16 [ 0, %entry ], [ %inc8, %for.cond.cleanup3 ]
+ %idxprom = zext i16 %i.02 to i32
+ %arrayidx = getelementptr [18 x i16], ptr addrspace(21) null, i32 %idxprom
+ br label %for.body4
+
+for.cond.cleanup:
+ ret i16 0
+
+for.cond.cleanup3:
+ %inc8 = add i16 %i.02, 1
+ %exitcond3.not = icmp eq i16 %inc8, 0
+ br i1 %exitcond3.not, label %for.cond.cleanup, label %for.cond1.preheader
+
+for.body4:
+ %j.01 = phi i16 [ 0, %for.cond1.preheader ], [ %inc.2, %for.body4 ]
+ %idxprom5 = zext i16 %j.01 to i32
+ %arrayidx6 = getelementptr i16, ptr addrspace(21) %arrayidx, i32 %idxprom5
+ store i16 0, ptr addrspace(21) %arrayidx6, align 1
+ %inc.2 = add i16 %j.01, 1
+ %exitcond.not.2 = icmp eq i16 %inc.2, 18
+ br i1 %exitcond.not.2, label %for.cond.cleanup3, label %for.body4
+}
|
mikaelholmen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and I've verified that this solves the crash I saw with #164798
Thanks!
Fix the assertion failure after llvm#164798. The issue is that the comparison `Sizes.back() == ElementSize` can fail when their types are different. We should cast them to the wider type before the comparison.
Fix the assertion failure after llvm#164798. The issue is that the comparison `Sizes.back() == ElementSize` can fail when their types are different. We should cast them to the wider type before the comparison.
Fix the assertion failure after #164798. The issue is that the comparison
Sizes.back() == ElementSizecan fail when their types are different. We should cast them to the wider type before the comparison.