[llvm] [LAA] Fix off-by-EltSize in negative-step deref bounds check (PR #211964)
Aleksandr Popov via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 2 16:08:58 PDT 2026
https://github.com/aleks-tmb updated https://github.com/llvm/llvm-project/pull/211964
>From d8a8fb54108eec8f47bfc98235eaed887cb68e29 Mon Sep 17 00:00:00 2001
From: Aleksandr Popov <apopov at azul.com>
Date: Mon, 27 Jul 2026 00:08:52 +0000
Subject: [PATCH] [LAA] Fix off-by-EltSize bounds in reverse-loop deref no-wrap
check
evaluatePtrAddRecAtMaxBTCWillNotWrap used AR->getStart() as the
lowest accessed address; for a negative step it is the *highest*.
Both safety checks on the reverse-loop branch inherited this and
were off by EltSize:
* No-underflow: added an extra EltSize of slack, rejecting
reverse loops whose last iteration lands on the base pointer.
* Deref-end: dropped the size of the top access, accepting
loops whose top read spills past DerefBytes.
Pick LowestAddr per step direction (AR->getStart() vs.
AR->evaluateAtIteration(MaxBTC, SE)). The range algebra downstream
becomes direction-agnostic and the per-direction MaxOffset branch
collapses into a single expression.
---
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 55 +++++++++----------
.../negative-step-deref-off-by-eltsize.ll | 12 ++--
2 files changed, 31 insertions(+), 36 deletions(-)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index c99d43dd1ccc2..9c1c79089c43d 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -208,8 +208,18 @@ static const SCEV *mulSCEVNoOverflow(const SCEV *A, const SCEV *B,
return SE.getMulExpr(A, B);
}
-/// Return true, if evaluating \p AR at \p MaxBTC cannot wrap, because \p AR at
+/// Return true if evaluating \p AR at \p MaxBTC cannot wrap, because \p AR at
/// \p MaxBTC is guaranteed inbounds of the accessed object.
+///
+/// The accessed byte range is [LowestOffset, LowestOffset + AccessedBytes),
+/// where
+/// AccessedBytes = MaxBTC * |Step| + EltSize,
+/// LowestOffset = smallest byte offset from StartPtr any iteration reaches.
+///
+/// The function returns true only when both safety invariants hold, regardless
+/// of step direction:
+/// 1. LowestOffset >= 0 (no access below StartPtr)
+/// 2. LowestOffset + AccessedBytes <= DerefBytes (no access past the region)
static bool evaluatePtrAddRecAtMaxBTCWillNotWrap(
const SCEVAddRecExpr *AR, const SCEV *MaxBTC, const SCEV *EltSize,
ScalarEvolution &SE, const DataLayout &DL, DominatorTree *DT,
@@ -267,15 +277,15 @@ static bool evaluatePtrAddRecAtMaxBTCWillNotWrap(
Step = SE.getNoopOrSignExtend(Step, WiderTy);
MaxBTC = SE.getNoopOrZeroExtend(MaxBTC, WiderTy);
- // For the computations below, make sure they don't unsigned wrap.
- // FIXME: for a negative step the lowest accessed address is not
- // AR->getStart() but AR->evaluateAtIteration(MaxBTC, SE); the check below
- // therefore compares StartPtr against the highest accessed address instead
- // of the lowest.
- if (!SE.isKnownPredicate(CmpInst::ICMP_UGE, AR->getStart(), StartPtr))
+ const SCEV *LowestAddr = IsKnownNonNegative
+ ? static_cast<const SCEV *>(AR->getStart())
+ : AR->evaluateAtIteration(MaxBTC, SE);
+ // Lower-bound safety check: the lowest accessed address must not fall below
+ // StartPtr.
+ if (!SE.isKnownPredicate(CmpInst::ICMP_UGE, LowestAddr, StartPtr))
return false;
- const SCEV *StartOffset = SE.getNoopOrZeroExtend(
- SE.getMinusSCEV(AR->getStart(), StartPtr), WiderTy);
+ const SCEV *LowestOffset =
+ SE.getNoopOrZeroExtend(SE.getMinusSCEV(LowestAddr, StartPtr), WiderTy);
if (!LoopGuards)
LoopGuards.emplace(ScalarEvolution::LoopGuards::collect(AR->getLoop(), SE));
@@ -304,27 +314,12 @@ static bool evaluatePtrAddRecAtMaxBTCWillNotWrap(
if (!AccessedBytes)
return false;
- // Compute MaxOffset per direction: exclusive upper offset of the
- // accessed range.
- const SCEV *MaxOffset;
- if (IsKnownNonNegative) {
- MaxOffset = addSCEVNoOverflow(StartOffset, AccessedBytes, SE);
- if (!MaxOffset)
- return false;
- DerefBytesSCEV = SE.applyLoopGuards(DerefBytesSCEV, *LoopGuards);
- } else {
- // FIXME: two independent off-by-EltSize bugs on this branch:
- // 1. StartOffset here is actually the HIGHEST offset, because it is
- // computed from AR->getStart() rather than
- // AR->evaluateAtIteration(MaxBTC, SE) (see FIXME above).
- // 2. The lower check is over-strict by EltSize and the upper is
- // under-counted by EltSize.
- assert(SE.isKnownNegative(Step) && "must be known negative");
- if (!SE.isKnownPredicate(CmpInst::ICMP_SGE, StartOffset, AccessedBytes))
- return false;
- MaxOffset = StartOffset;
- }
- // MaxOffset must not exceed the deref-region end.
+ // Exclusive upper offset of the accessed range.
+ const SCEV *MaxOffset = addSCEVNoOverflow(LowestOffset, AccessedBytes, SE);
+ if (!MaxOffset)
+ return false;
+ DerefBytesSCEV = SE.applyLoopGuards(DerefBytesSCEV, *LoopGuards);
+ // Upper-bound safety check: MaxOffset must not exceed the deref-region end.
return SE.isKnownPredicate(CmpInst::ICMP_ULE, MaxOffset, DerefBytesSCEV);
}
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/negative-step-deref-off-by-eltsize.ll b/llvm/test/Analysis/LoopAccessAnalysis/negative-step-deref-off-by-eltsize.ll
index 1a53fba949749..872a948de6cb5 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/negative-step-deref-off-by-eltsize.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/negative-step-deref-off-by-eltsize.ll
@@ -4,7 +4,7 @@
; Reverse loop loading 4 i32 elements whose access range exactly fills the
; dereferenceable region (deref(16), reads bytes [0, 16)).
;
-; TODO: LAA should recognise that this AR fits within the deref
+; LAA should recognise that this AR fits within the deref
; region and produce tight bounds (Low: %A, High: %A + 16).
;
; Pseudocode:
@@ -28,10 +28,10 @@ define void @reverse_reaches_base(ptr dereferenceable(16) %A, ptr dereferenceabl
; CHECK-NEXT: %gep.A = getelementptr inbounds i32, ptr %A, i64 %iv
; CHECK-NEXT: Grouped accesses:
; CHECK-NEXT: Group GRP0:
-; CHECK-NEXT: (Low: (-4 + inttoptr (i64 -1 to ptr))<nsw> High: (16 + %B)<nuw>)
+; CHECK-NEXT: (Low: %B High: (16 + %B)<nuw>)
; CHECK-NEXT: Member: {(12 + %B)<nuw>,+,-4}<nw><%loop>
; CHECK-NEXT: Group GRP1:
-; CHECK-NEXT: (Low: (-4 + inttoptr (i64 -1 to ptr))<nsw> High: (16 + %A)<nuw>)
+; CHECK-NEXT: (Low: %A High: (16 + %A)<nuw>)
; CHECK-NEXT: Member: {(12 + %A)<nuw>,+,-4}<nw><%loop>
; CHECK-EMPTY:
; CHECK-NEXT: Non vectorizable stores to invariant address were not found in loop.
@@ -67,7 +67,7 @@ exit.done:
; The top i32 read at byte 13 covers [13, 17), but deref(16) only
; guarantees [0, 16) — bytes at/after 16 may or may not be dereferenceable.
;
-; TODO: LAA must not assume the AR fits in the deref region and should
+; LAA must not assume the AR fits in the deref region and should
; fall back to the wide low bound.
;
; Pseudocode:
@@ -90,10 +90,10 @@ define void @reverse_top_spills(ptr dereferenceable(16) %A, ptr dereferenceable(
; CHECK-NEXT: %gep.A = getelementptr inbounds i8, ptr %A, i64 %iv
; CHECK-NEXT: Grouped accesses:
; CHECK-NEXT: Group GRP0:
-; CHECK-NEXT: (Low: (5 + %B)<nuw> High: (17 + %B))
+; CHECK-NEXT: (Low: (-4 + inttoptr (i64 -1 to ptr))<nsw> High: (17 + %B))
; CHECK-NEXT: Member: {(13 + %B)<nuw>,+,-4}<nw><%loop>
; CHECK-NEXT: Group GRP1:
-; CHECK-NEXT: (Low: (5 + %A)<nuw> High: (17 + %A))
+; CHECK-NEXT: (Low: (-4 + inttoptr (i64 -1 to ptr))<nsw> High: (17 + %A))
; CHECK-NEXT: Member: {(13 + %A)<nuw>,+,-4}<nw><%loop>
; CHECK-EMPTY:
; CHECK-NEXT: Non vectorizable stores to invariant address were not found in loop.
More information about the llvm-commits
mailing list