This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Check for HardwareLoop Latch ExitBlock
ClosedPublic

Authored by samparker on Jun 14 2019, 8:37 AM.

Details

Summary

The HardwareLoops pass finds exit blocks with a scevable exit count. If the target specifies to update the loop counter in a register, through a phi, we need to ensure that the exit block is a latch so that we can insert the phi with the correct value for the incoming edge.

Diff Detail

Repository
rL LLVM

Event Timeline

samparker created this revision.Jun 14 2019, 8:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 14 2019, 8:37 AM
Herald added a subscriber: javed.absar. · View Herald Transcript
lebedev.ri resigned from this revision.Jun 14 2019, 4:13 PM
dmgreen added inline comments.Jun 15 2019, 1:14 AM
lib/CodeGen/HardwareLoops.cpp
247 ↗(On Diff #204773)

Maybe "RequiresLoopExitingLatch" as a more descriptive name?

lib/Target/ARM/ARMTargetTransformInfo.cpp
706 ↗(On Diff #204773)

Is it worth just checking for the LoopExitingLatch here, as opposed to adding a parameter for it?

(Put another way, why would one be a parameter and the other be something for the backend to figure out?)

samparker updated this revision to Diff 205013.Jun 17 2019, 3:51 AM
samparker retitled this revision from [CodeGen] Add RequiresLatch to HardwareLoopInfo to [CodeGen] Check for HardwareLoop Latch ExitBlock.
samparker edited the summary of this revision. (Show Details)
samparker removed a reviewer: lebedev.ri.

Cheers Dave. I've removed the ExitBlock check in the ARM backend. Also, currently we'd need CounterInReg == RequiresLoopLatchExit so I've just switched to querying CounterInReg instead of introducing a new parameter. If, later, there's another target which has a separate requirement, then we can re-add the option.

Am I correct in saying that this loop will find the first exiting block that dominates all the latches in the loop (with some other conditions like a loop invariant non zero exit count). Plus is now also a latch?

But the loop may have other exits and other latches?

The "other exits" sounds like it should be OK for arm low overhead loops. At least I can't think of a reason right now why they are not OK. For vector tail predicated loops I imagine that you would need to ensure that a "LCTP" is placed on the other exits, to ensure the tail predicate doesn't pollute extra instructions after the loop.

I'm not sure about the "other latches" part. Is that something that's OK? I imagine it doesn't come up very often.

dmgreen accepted this revision.Jun 17 2019, 4:57 AM

Actually, looking at it, because we can compute the BETC, we will only have a single latch. Ignore me about that bit.

For scalar low overhead loops, this LGTM. Vector loops we are looking into later, as far as I understand.

This revision is now accepted and ready to land.Jun 17 2019, 4:57 AM
This revision was automatically updated to reflect the committed changes.