Skip to content

Conversation

@kasuga-fj
Copy link
Contributor

@kasuga-fj kasuga-fj commented Oct 23, 2025

This patch stops propagating nowrap flags unconditionally. In the modified part, when we have an addrec {%a,+,%b} where %b is known to be negative, we create a new addrec {%a,+,(-1 * %b)}. When creating it, the nowrap flags are transferred from the original addrec to the new one without any checks, which is generally incorrect. Since the nowrap property is not essential for this analysis, it would be better to drop it to avoid potential bugs.

@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Oct 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 23, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

Changes

This patch stops propagating nowrap flags unconditionally. In the modified part, when we have an addrec {%a,+,%b} where %b is known to be negative, we create a new addrec {%a,+,(-1 * %b)}. In this case, the nowrap flags are transferred from the original addrec to the new one without any checks, which is generally incorrect. Since the nowrap property is not essential for this analysis, it would be better to drop it to avoid potential bugs.


Full diff: https://github.com/llvm/llvm-project/pull/164796.diff

1 Files Affected:

  • (modified) llvm/lib/Analysis/LoopCacheAnalysis.cpp (+3-4)
diff --git a/llvm/lib/Analysis/LoopCacheAnalysis.cpp b/llvm/lib/Analysis/LoopCacheAnalysis.cpp
index 050c32707596a..424a7fe3721bb 100644
--- a/llvm/lib/Analysis/LoopCacheAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopCacheAnalysis.cpp
@@ -436,10 +436,9 @@ bool IndexedReference::delinearize(const LoopInfo &LI) {
       const SCEV *StepRec = AccessFnAR ? AccessFnAR->getStepRecurrence(SE) : nullptr;
 
       if (StepRec && SE.isKnownNegative(StepRec))
-        AccessFn = SE.getAddRecExpr(AccessFnAR->getStart(),
-                                    SE.getNegativeSCEV(StepRec),
-                                    AccessFnAR->getLoop(),
-                                    AccessFnAR->getNoWrapFlags());
+        AccessFn = SE.getAddRecExpr(
+            AccessFnAR->getStart(), SE.getNegativeSCEV(StepRec),
+            AccessFnAR->getLoop(), SCEV::NoWrapFlags::FlagAnyWrap);
       const SCEV *Div = SE.getUDivExactExpr(AccessFn, ElemSize);
       Subscripts.push_back(Div);
       Sizes.push_back(ElemSize);

@kasuga-fj kasuga-fj requested review from fhahn and nikic October 23, 2025 11:20
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but this change does not look NFC to me?

@kasuga-fj
Copy link
Contributor Author

LGTM, but this change does not look NFC to me?

Yes, you're right, this is not NFC. Thanks.

@kasuga-fj kasuga-fj changed the title [LoopCacheAnalysis] Drop incorrect nowrap flags from addrec (NFCI) [LoopCacheAnalysis] Drop incorrect nowrap flags from addrec Oct 23, 2025
@kasuga-fj kasuga-fj merged commit c6073d7 into llvm:main Oct 23, 2025
12 checks passed
@kasuga-fj kasuga-fj deleted the loopcacheanalysis-drop-nowrap branch October 23, 2025 15:32
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
)

This patch stops propagating nowrap flags unconditionally. In the
modified part, when we have an addrec `{%a,+,%b}` where `%b` is known to
be negative, we create a new addrec `{%a,+,(-1 * %b)}`. When creating
it, the nowrap flags are transferred from the original addrec to the new
one without any checks, which is generally incorrect. Since the nowrap
property is not essential for this analysis, it would be better to drop
it to avoid potential bugs.
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
)

This patch stops propagating nowrap flags unconditionally. In the
modified part, when we have an addrec `{%a,+,%b}` where `%b` is known to
be negative, we create a new addrec `{%a,+,(-1 * %b)}`. When creating
it, the nowrap flags are transferred from the original addrec to the new
one without any checks, which is generally incorrect. Since the nowrap
property is not essential for this analysis, it would be better to drop
it to avoid potential bugs.
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
)

This patch stops propagating nowrap flags unconditionally. In the
modified part, when we have an addrec `{%a,+,%b}` where `%b` is known to
be negative, we create a new addrec `{%a,+,(-1 * %b)}`. When creating
it, the nowrap flags are transferred from the original addrec to the new
one without any checks, which is generally incorrect. Since the nowrap
property is not essential for this analysis, it would be better to drop
it to avoid potential bugs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants