[test] A test case for D146958

[pawosm01]

Summary

This commit introduces a test for the change introduced by the
[SCEV] Do not plant SCEV checks unnecessarily commit,
D146958.

[fhahn]

llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll

8

entry?

11

Is this load needed or can the value directly be passed as argument?

15

Please shorten the block names and try to make them more descriptive, this will make the test easier to read. E.g. IIUC this could be something like loop1.header.

18

Is this branch needed here?

21

Is this load needed or can the value directly be passed as argument?

26

this could be loop2.header.

31

is this branch needed?

33

This could be loop3.header. Is a 3 level loop nest needed for the test? Would a 2 level loop nest be sufficient?

46

loop2.latch?

51

loop1.latch

56

exit?

[pawosm01]

Failed as expected.

[pawosm01]

@fhahn Thanks for your comments. If you don't mind, I will address them in the post-review changes of D146958 in coming day(s).

[pawosm01]

llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll

31

Yes

[pawosm01]

Test updated as per the reviewer request.

[pawosm01]

seems I had a wrong understanding of the reviewer's intent.

[pawosm01]

I'll try to build a relation between D146974 and D146958.

[xbolva00]

llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll

2

This test needs to pass with current master.

Tests are commited as first, so things would be broken.

[pawosm01]

Following reviewer's advice.

[pawosm01]

wrong diff file picked; this is the correct one.

[xbolva00]

Looks OK now. @fhahn ?

[david-arm]

LGTM! It looks like you've addressed @fhahn's comments.

[fhahn]

llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll

3

could you add a brief comment saying what this test is intended to check?

15

Could you add a new function that's basically the same as @foo but has %arrayidx0 in %loop2.header?

[pawosm01]

llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll

15

Could you add a new function that's basically the same as @foo but has %arrayidx0 in %loop2.header?

This will not expose the same behavior of the D146958 patch we care about in this test (namely, vector.scevcheck will remain).

[pawosm01]

a short comment added as requested

[pawosm01]

Hi @fhahn, are you happy with the current shape?

[fhahn]

llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll

15

Right, but that's only with the current version of the patch; it should be relatively straightforward to also handle this case (https://reviews.llvm.org/D146958#inline-1439048). In any case, it won't hurt to have negative test cases as well.

[pawosm01]

Extended with one more variant of the same test case, as requested.

[pawosm01]

llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll

15

Hi Florian, your suggestion from the patch review has worked, hence I've modified this test file accordingly. I'd like to have your explicit approval before I merge this one.

[fhahn]

LGTM, thanks!

llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll

+; RUN: opt -passes=loop-vectorize -force-vector-width=2 -force-vector-interleave=1 -S < %s | FileCheck %s
+
+define void @foo(ptr %pout, ptr %pin, i64 %val0, i64 %val1, i64 %val2) {
+; CHECK-LABEL: @foo(
+; FIXME: CHECK below needs to be changed to CHECK-NOT to confirm the change.
+; CHECK: vector.scevcheck
+; CHECK: vector.body
+entry:
+  %0 = getelementptr double, ptr %pin, i64 %val0
+  br label %loop1.header
+
+loop1.header:                                     ; preds = %loop1.latch, %entry
+  %i = phi i64 [ %i.next, %loop1.latch ], [ 0, %entry ]
+  %mul0 = mul nsw i64 %i, %val2
+  %arrayidx0 = getelementptr inbounds double, ptr %0, i64 %mul0
+  %mul1 = mul nsw i64 %i, %val1
+  br label %loop2.header
+
+loop2.header:                                     ; preds = %loop1.header, %loop2.header
+  %j = phi i64 [ 0, %loop1.header ], [ %j.next, %loop2.header ]
+  %1 = load double, ptr %arrayidx0, align 8
+  %arrayidx1 = getelementptr inbounds double, ptr %0, i64 %j
+  %2 = load double, ptr %arrayidx1, align 8
+  %sum = fadd contract double %1, %2
+  %3 = getelementptr double, ptr %pout, i64 %mul1
+  %arrayidx2 = getelementptr inbounds double, ptr %3, i64 %j
+  store double %sum, ptr %arrayidx2, align 8
+  %j.next = add nuw nsw i64 %j, 1
+  %cmp = icmp slt i64 %j.next, %val1
+  br i1 %cmp, label %loop2.header, label %loop1.latch
+
+loop1.latch:                                      ; preds = %loop2.header
+  %i.next = add nuw nsw i64 %i, 1
+  %exitcond = icmp eq i64 %i.next, %val1
+  br i1 %exitcond, label %exit, label %loop1.header
+
+exit:                                             ; preds = %loop1.latch
+  ret void
+}