Peeling assumed this doesn't happen, but didn't check it.
This fixes PR32178.
Peeling assumed this doesn't happen, but didn't check it.
This fixes PR32178.
Looks good to me, but, as you noticed in the PR - do we want to work with such loops at all? Did you check what happens if we just bail out on such loops?
Michael
No, but I'm tempted to, because I think we have more bugs related to irreducible control flow that this patch doesn't fix.
Do you know if we already have a utility somewhere that detects this? There's no way to get this information out of LoopInfo that I'm aware of.
LV rolls its own ( hasCyclesInLoopBody() ), which would be fine for my original use case, but r296898 started handling non-innermost loops.
Do you know if we already have a utility somewhere that detects this?
Can we require the loop to be in a simplified form and its latch to be exiting? Will it suffice?
Maybe?
It will certainly squash this bug, not sure about the others.
We already sort of try to do this - UnrollLoop() bails if the latch is not conditional. I assume the expectation is that one of the edges is the backedge, and the other is the exiting edge, but I don't think this is actually enforced, and it looks like unrolling (as opposed to peeling) manages to deal with it.
This version looks good to me too.
MichaelZ
PS: Can we have a similar problem in the regular loop unrolling?
test/Transforms/LoopUnroll/peel-loop-irreducible.ll | ||
---|---|---|
36–37 ↗ | (On Diff #92045) | I'd prefer to have LCSSA-ed version of the loop in the test. And, by the way, do we need %sum, %plus and %incsum at all? Maybe remove them to make the test even smaller? |
Thanks!
Re the PS - I don't know.
As I wrote above - UnrollLoop() verifies the latch is conditional, but doesn't verify the two edges are actually going to the header and the exit block. But we haven't actually *seen* any issues with this, as far as I know.
test/Transforms/LoopUnroll/peel-loop-irreducible.ll | ||
---|---|---|
36–37 ↗ | (On Diff #92045) | Sure, I'll see if it minimizes more. |