"Trip Count for Loop [...] Changed! Old: 6 New: 0 Delta: 6" with simple-loop-unswitch and -verify-scev

[mikaelholmen]

llvm commit: 6568062
Reproduce with opt -S -passes="function(print<scalar-evolution>,loop-mssa(simple-loop-unswitch))" -verify-scev bbi-99125_2.ll
Result:

Trip Count for Loop at depth 1 containing: %for.cond<header><exiting>,%for.body,%for.cond25.i<exiting>,%func_32.exit.i,%func_7.exit<latch>
    Loop at depth 2 containing: %for.cond25.i<header><exiting>,%func_32.exit.i<latch><exiting>
 Changed!
Old: 6
New: 0
Delta: 6

(Thanks to @nikic for reducing the testcase in
#106075 (comment) )

bbi-99125_2.ll.gz

[mikaelholmen]

This still happens on trunk (e4f3e9a): https://godbolt.org/z/zK71fvM7q

[sjoerdmeijer]

Thanks for checking.
Here's a slightly more minimal reproducer (and a bit of renaming): https://godbolt.org/z/34cn531sx

[nikic]

I only looked at @sjoerdmeijer's reduced variant, but that looks like a false positive to me. The loop contains an interior infinite loop, which means that both the trip counts 0 and 1 are correct for the outer loop. (The trip count does not take abnormal exits or diverging control flow into account.)

This verification also has other false positives where the loop is known dead.

I'm not really sure how to make this more reliable -- I guess for this specific issue we could add a check that all the sub-loops also have a known trip count and otherwise skip it?

[sjoerdmeijer]

I only looked at @sjoerdmeijer's reduced variant, but that looks like a false positive to me. The loop contains an interior infinite loop, which means that both the trip counts 0 and 1 are correct for the outer loop. (The trip count does not take abnormal exits or diverging control flow into account.)

This verification also has other false positives where the loop is known dead.

It's a bit noisy when I enable expensive checks in testing, which I do less frequent because it takes longer, but am quite keen on making it less noisy as the expensive checks are finding quite some issues.

I'm not really sure how to make this more reliable -- I guess for this specific issue we could add a check that all the sub-loops also have a known trip count and otherwise skip it?

That sounds very reasonable. The problem here looks actually very similar to the interchange issue #163954 that I've just picked up. Also in that case there are interior infinite loops (br true branches to the same block) that confuses things. It looks like this pattern is quite common for fuzzer generated tests. But anyway, I don't think interchange should attempt to interchange these kind of loops, which is what it currently is attempting, so similarly I would like to do a check on the loops.

I was going to look at the interchange issue first, but don't mind looking at this one after that, if someone else doesn't get there first.