-
Notifications
You must be signed in to change notification settings - Fork 15k
[NFC][LV] Improve ee with sideeffects legality test #158275
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
[NFC][LV] Improve ee with sideeffects legality test #158275
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: Graham Hunter (huntergr-arm) ChangesAddressing postcommit comments for 54fc536 Full diff: https://github.com/llvm/llvm-project/pull/158275.diff 1 Files Affected:
diff --git a/llvm/test/Transforms/LoopVectorize/early_exit_store_legality.ll b/llvm/test/Transforms/LoopVectorize/early_exit_store_legality.ll
index 4226c5d9e650b..0f5f8876b6ef1 100644
--- a/llvm/test/Transforms/LoopVectorize/early_exit_store_legality.ll
+++ b/llvm/test/Transforms/LoopVectorize/early_exit_store_legality.ll
@@ -604,36 +604,31 @@ exit:
;; ICE was caused by assert for the load used in the uncountable exit condition
;; being guaranteed to execute.
-@e = external addrspace(21) global [4 x i8]
-define void @crash_conditional_load_for_uncountable_exit() {
+@ee.global = external global [4 x i8]
+define void @crash_conditional_load_for_uncountable_exit(ptr dereferenceable(40) noalias %store.area) {
; CHECK-LABEL: LV: Checking a loop in 'crash_conditional_load_for_uncountable_exit'
; CHECK: LV: Not vectorizing: Load for uncountable exit not guaranteed to execute.
entry:
- br label %cont
-
-handler.out_of_bounds:
- unreachable
+ br label %for.body
-cont:
- %h.06 = phi i64 [ 0, %entry ], [ %inc, %a.exit ]
- %arrayidx = getelementptr i8, ptr addrspace(21) @e, i64 %h.06
- br i1 false, label %cont1, label %handler.type_mismatch
+for.body:
+ %iv = phi i64 [ 0, %entry ], [ %iv.next, %for.inc ]
+ %ee.addr = getelementptr i8, ptr @ee.global, i64 %iv
+ br i1 false, label %ee.block, label %invalid.block
-handler.type_mismatch:
- unreachable
+ee.block:
+ %ee.val = load i8, ptr %ee.addr, align 1
+ store i16 0, ptr %store.area, align 2
+ %ee.cmp = icmp eq i8 %ee.val, 0
+ br i1 %ee.cmp, label %for.inc, label %invalid.block
-cont1:
- %0 = load i8, ptr addrspace(21) %arrayidx, align 1
- store i16 0, ptr null, align 2
- %cmp.not.i.i = icmp eq i8 %0, 0
- br i1 %cmp.not.i.i, label %a.exit, label %if.then.i.i
+for.inc:
+ %iv.next = add nuw nsw i64 %iv, 1
+ %counted.cond = icmp eq i64 %iv.next, 10
+ br i1 %counted.cond, label %invalid.block, label %for.body
-if.then.i.i:
+invalid.block:
unreachable
-
-a.exit:
- %inc = add i64 %h.06, 1
- br i1 true, label %handler.out_of_bounds, label %cont
}
|
Fixes an ICE reported on PR #145663, as an assert was found to be reachable with a specific combination of unreachable blocks.
|
Cancelled the CI checks due to an unrelated failure; will rebase later. |
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, thanks
| for.body: | ||
| %iv = phi i64 [ 0, %entry ], [ %iv.next, %for.inc ] | ||
| %ee.addr = getelementptr i8, ptr @ee.global, i64 %iv | ||
| br i1 false, label %ee.block, label %invalid.block |
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.
Do we also hit the crash with a condition passed as argument? If so, would be good to add that as additional test coverage as well.
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.
Assuming you mean having the address come from an argument instead of the global, yes. Added.
Or were you referring to the i1 false condition?
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.
Providing the condition via argument dropped out earlier with Cannot vectorize early exit loop with more than one early exit. Disabling that check bails out at Cannot vectorize potentially faulting early exit loop. Disabling that check as well will give us the crash. So I'll add it as part of the second test, but I don't think we would have hit it before.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/40012 Here is the relevant piece of the build log for the reference |
Addressing postcommit comments for 54fc536