Page MenuHomePhabricator

[MLIR] Add const iterator and isLegalToHoist to Block
AbandonedPublic

Authored by wsmoses on Jun 22 2021, 2:33 PM.

Details

Summary

It is desirable to use several implementation-agnostic utilities for control flow graphs (such as loops) implemented in LLVM. This PR adds two common functions (a const iterator, and isLegalToHoist) to better allow MLIR blocks work with such API's

Diff Detail

Event Timeline

wsmoses created this revision.Jun 22 2021, 2:33 PM
wsmoses requested review of this revision.Jun 22 2021, 2:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2021, 2:33 PM
rriddle requested changes to this revision.Jun 22 2021, 2:34 PM
rriddle added inline comments.
mlir/include/mlir/IR/Block.h
261

What is the justification here? This seems like the wrong place for this.

This revision now requires changes to proceed.Jun 22 2021, 2:34 PM
rriddle added inline comments.Jun 22 2021, 2:35 PM
mlir/include/mlir/IR/Block.h
133

This const API goes against the MLIR rationale: https://mlir.llvm.org/docs/Rationale/UsageOfConst/

Can you elaborate on the "why" of this patch? It isn't clear what exactly you are trying to do.

wsmoses added inline comments.Jun 22 2021, 2:38 PM
mlir/include/mlir/IR/Block.h
261

This function, verbatim, is required by LoopInfo (https://llvm.org/doxygen/LoopInfoImpl_8h_source.html). I'm open to other suggestions.

template <class BlockT, class LoopT>
BlockT *LoopBase<BlockT, LoopT>::getLoopPreheader() const {
  assert(!isInvalid() && "Loop not in a valid state!");
  // Keep track of nodes outside the loop branching to the header...
  BlockT *Out = getLoopPredecessor();
  if (!Out)
    return nullptr;
 
  // Make sure we are allowed to hoist instructions into the predecessor.
  if (!Out->isLegalToHoistInto())
    return nullptr;
 
  // Make sure there is only one exit out of the preheader.
  typedef GraphTraits<BlockT *> BlockTraits;
  typename BlockTraits::ChildIteratorType SI = BlockTraits::child_begin(Out);
  ++SI;
  if (SI != BlockTraits::child_end(Out))
    return nullptr; // Multiple exits from the block, must not be a preheader.
 
  // The predecessor has exactly one successor, so it is a preheader.
  return Out;
}

I have a subsequent PR that uses llvm LoopInfo to create SCF loops out of CFG loops (hence the need for loop analysis)

rriddle added inline comments.Jun 22 2021, 2:41 PM
mlir/include/mlir/IR/Block.h
261

Have you considered changing the structure of those analyses instead of adding these methods to MLIR (e.g. via traits)? I'm a bit wary of adding stub functions to the core IR to satisfy the needs of a templated analysis.

wsmoses abandoned this revision.Jun 23 2021, 6:31 PM

Working around elsewhere.