-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[LAA] Pass maximum stride to isSafeDependenceDistance. #90036
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
Changes from all commits
fcec466
54a94b7
5db9dd2
c958e24
77f42ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,10 +8,6 @@ define void @forward_dep_known_safe_due_to_backedge_taken_count(ptr %A) { | |
; CHECK-NEXT: loop: | ||
; CHECK-NEXT: Memory dependences are safe | ||
; CHECK-NEXT: Dependences: | ||
; CHECK-NEXT: Forward: | ||
; CHECK-NEXT: %l = load i32, ptr %gep.mul.2, align 4 -> | ||
; CHECK-NEXT: store i32 %add, ptr %gep, align 4 | ||
; CHECK-EMPTY: | ||
; CHECK-NEXT: Run-time memory checks: | ||
; CHECK-NEXT: Grouped accesses: | ||
; CHECK-EMPTY: | ||
|
@@ -80,13 +76,8 @@ exit: | |
define void @unknown_dep_known_safe_due_to_backedge_taken_count(ptr %A) { | ||
; CHECK-LABEL: 'unknown_dep_known_safe_due_to_backedge_taken_count' | ||
; CHECK-NEXT: loop: | ||
; CHECK-NEXT: Report: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop | ||
; CHECK-NEXT: Unknown data dependence. | ||
; CHECK-NEXT: Memory dependences are safe | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be wrong due to my previous note: If the backedge taken count is not known then there is no safe distance between the pointers (unless the stride is 0, diferent from not computable) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I was confused here why a previously unsafe dep became safe when the title says There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes exactly, the test was specifically added for this PR, although I should probably have added a TODO.... |
||
; CHECK-NEXT: Dependences: | ||
; CHECK-NEXT: Unknown: | ||
; CHECK-NEXT: %l = load i32, ptr %gep, align 4 -> | ||
; CHECK-NEXT: store i32 %add, ptr %gep.mul.2, align 4 | ||
; CHECK-EMPTY: | ||
; CHECK-NEXT: Run-time memory checks: | ||
; CHECK-NEXT: Grouped accesses: | ||
; CHECK-EMPTY: | ||
|
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.
I think StrideA/StrideB are set to 0 if the stride could not be computed (and then indistinguishable from no stride;
getPtrStride
actually returns anstd::optional
but it is lost somewhere). I think we must ensure that neither StrideA nor StrideB are zero.Or even better: Keep passing on the
std::optional
fromgetPtrStride
.Or even better: Use
StrideA
orStrideB
depending on whetherDist
is negative or positive.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.
hey must be non-zero for now which should be ensured here
llvm-project/llvm/lib/Analysis/LoopAccessAnalysis.cpp
Line 2002 in 5db9dd2
That sounds like a good improvement that should help additional cases. Would you prefer this pulled in this patch or separate? In any case, I'll first need to add additional test cases to cover this
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.
I see. Thanks for pointing me there.