LoopVectorize: add test for crash in #72969

[artagnon]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

Changes

---

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

1 Files Affected:

• (added) llvm/test/Transforms/LoopVectorize/pr72969.ll (+26)

diff --git a/llvm/test/Transforms/LoopVectorize/pr72969.ll b/llvm/test/Transforms/LoopVectorize/pr72969.ll
new file mode 100644
index 000000000000000..47d3efc10fc4c49
--- /dev/null
+++ b/llvm/test/Transforms/LoopVectorize/pr72969.ll
@@ -0,0 +1,26 @@
+; RUN: not --crash opt -passes=loop-vectorize -S < %s
+
+target triple = "x86_64-unknown-linux-gnu"
+
+@h = global i32 0, align 4
+
+define void @test(ptr %p) {
+entry:
+  br label %for.body
+
+for.body:
+  %idx.ext.merge = phi i64 [ 1, %entry ], [ %idx, %for.body ]
+  %inc.merge = phi i16 [ 1, %entry ], [ %inc, %for.body ]
+  %idx.merge = phi i64 [ 0, %entry ], [ %idx.ext.merge, %for.body ]
+  %add = shl i64 %idx.merge, 1
+  %arrayidx = getelementptr [1 x i32], ptr %p, i64 0, i64 %add
+  %0 = load i32, ptr %arrayidx, align 4
+  %inc = add i16 %inc.merge, 1
+  %idx = zext i16 %inc to i64
+  %gep = getelementptr i32, ptr %p, i64 %idx
+  %cmp = icmp ugt ptr %gep, @h
+  br i1 %cmp, label %exit, label %for.body
+
+exit:
+  ret void
+}

[nikic]

We don't use pre-commit tests for crashes. You can include it directly in the fix PR.

[david-arm]

What's the best thing to do with this PR? The test seems fine, but if it should be added as part of the actual fix does it make sense to close this and just review #74456?

[artagnon]

Hi @david-arm,

Sorry I'm behind on my PRs: I'm currently on vacation, due to return to work in mid-March.

As for this PR, since the test is fine, I think we can merge it as-is, to document the failure. I'll try to pick up #74456 soon and complete it. What do you think?

[david-arm]

LGTM!

[david-arm]

Hi @david-arm,

Sorry I'm behind on my PRs: I'm currently on vacation, due to return to work in mid-March.

As for this PR, since the test is fine, I think we can merge it as-is, to document the failure. I'll try to pick up #74456 soon and complete it. What do you think?

Yeah, seems reasonable. And no rush to finish the other patch - enjoy your vacation!

[artagnon]

I got a buildbot failure post-merge, although the native CI seems to have passed it.

[nikic]

And this is why I told you not to pre-commit tests for crashes :) If you do this, you need to at least make it a REQUIRES: asserts test, to be reasonably confident that it will always crash.

[artagnon]

Thanks for the suggestion; will attempt a quick fix. [I really shouldn't have merged something while on vacation: this is what I get]