This is an archive of the discontinued LLVM Phabricator instance.

Describe resticted form of loops in the new loop terminogy documentation
AbandonedPublic

Authored by reames on Jul 24 2019, 5:41 PM.

Details

Summary

What do folks think of the reframing?

Diff Detail

Event Timeline

reames created this revision.Jul 24 2019, 5:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2019, 5:41 PM
reames updated this revision to Diff 211649.Jul 24 2019, 5:58 PM
reames retitled this revision from Describe loop simplify form in the new loop terminogy documentation to Describe resticted form of loops in the new loop terminogy documentation.
reames edited the summary of this revision. (Show Details)

Thought of a cleaner way to describe it.

reames updated this revision to Diff 211653.Jul 24 2019, 6:15 PM

Describe a few more restricted forms.

jdoerfert added inline comments.
docs/LoopTerminology.rst
148

Loops, assuming "natural loops identified by LoopInfo", can always be identified/distinguished by their header blocks regardless of the backedges. The situation is different for irreducible loops though.

Maybe I misunderstood what you wanted to say. Can you clarify, give an example?

Meinersbur requested changes to this revision.Jul 25 2019, 8:15 AM
Meinersbur added a subscriber: Meinersbur.
Meinersbur added inline comments.
docs/LoopTerminology.rst
138–139

I don't get this. Where can these predicates be used? More importantly, which pass ensures the restricted/normal/canonical forms?

I'd not describe a restricted loop forms in terms of an API call. That's what the merthod documentation/doxygen is for. If you are not a frequent user of the Loop class, it is not obvious what it means when e.g. getLoopLatch() return nullptr. The naive interpretation is "there is not latch". Rather it should be described as property of the CFG).

160

No; LoopSimplifyPass must have been added before or simplifyLoop run on it manually.

This revision now requires changes to proceed.Jul 25 2019, 8:15 AM
reames planned changes to this revision.Jul 25 2019, 2:21 PM
reames marked 3 inline comments as done.
reames added inline comments.
docs/LoopTerminology.rst
138–139

Paragraph A - Many passes bailout with these predicates. Mostly ensured by LoopSimplify, but other passes help too.

Paragraph B - I agree that the doxygen should have some of this, but the implications seem like possible overkill. I could be convinced that this needs to be rewritten as a comment patch though, what do others think?

148

Yep, this was a mistaken assumption on my part. I'll rephrase along the lines of https://reviews.llvm.org/D65299, once we're happy with the wording there.

160

Correct, but isn't it generally reasonable to assume that has happened by the point a loop pass runs? I'll admit, my wording was influenced by how things used to work in the old pass manager. Any suggestions on reframing?

Meinersbur added inline comments.Jul 26 2019, 7:27 AM
docs/LoopTerminology.rst
138–139

I think this is specific to pass/analysis implementations and does not belong into a document about loop terminology. Each transformation/pass should document its conditions, which may or may not include some of the ones described here.

I'd put about how to check for frequently used correctness conditions into the loop pass section of WritingAnLLVMPass, if at all. In this document, I'd describe the loop normal forms we have (LoopSimplify, LoopRotate, IndVarSimplify).

160

Loop normal forms are not implicit. It must be ensured by the pass (either by calling simplifyLoop or in the old pass manager this was possible by depending on e.g. LoopSimplify) or when adding the pass to the LoopPassManager (which might have been done already with some LoopPassManagers in the default pipeline, but this also depends on the position in the pipeline). At least to me this wording gives the impression that there is nothing to do if my pass depends on this condition.

169

Could you mention the reason why it is not a good idea to transform loops to make this hold?

reames abandoned this revision.Dec 3 2020, 3:03 PM