Removing Successor Operands

MLIR
1

Hi all,

This has been in my backlog for months now and I finally have time to devote to it. I’d love to hear thoughts.

tdlr; Successor operands are overly restrictive, create lots of headaches, and should be removed.

Generalizing Successor Operands

The current modeling of branch successor operands is much too rigid, and makes lots of things quite awkward. They are modeled with passthrough PHI Values that are required to exactly match the count and type of the argument list for the successor block. They are technically part of the operand list of an operation, but aren’t really controllable by the operation as they have a specific ordering/structure. They can’t be treated like arguments(in the ODS sense) to the operation. Another major problem is that the current implementation enforces that an SSA value already be materialized, making certain classes of terminators difficult(hacky) to implement. These are terminators that want to produce values only on specific edges, think switch_enum in SIL or CallBr/Invoke in LLVM.

This simplified a lot of things in the early days of MLIR, back when there were Instructions/Statements/MLFuncs/etc., but is growing increasingly awkward in the present day infrastructure. When I originally added support for terminator operations in those days, we didn’t have things like interfaces to allow for opaquely querying information about an operation. Now that we do, there is little incentive to keep the current structure and I argue that it is actively detrimental. As such, I propose that we detach how operands to a successor are specified and leave it up to the specific terminator operations. We can then use an interface BranchOpInterface to recover the API that the current infrastructure provides; e.g. erasing a successor operand, getting the operands to a specific successor, etc… This would essentially move successor operands in the current sense to just be normal operands to the operation.

I’ve gone ahead and uploaded a stack for review that showcases what this looks like, ending with https://reviews.llvm.org/D75318.

Some examples of hacky-logic because of the current state:

  • There are two matchAndRewrite methods for ConversionPattern to handle the case of successor operands.
    • Note that if you have a terminator with a variadic number of successors(e.g. switch), you have to implement both methods!
  • When building an operation the successors have to be added last, and the operands list needs to have null values between the success operand lists.

Thanks,

– River

2

This looks really fantastic River. I love how it removes numSuccessorOperands from BlockOperand which should shrinkify terminators. This seems like a big step forward in terms of generality too.

Do you think there is anything concerning about this? It seems great to me,

-Chris

3

Thanks, Chris.

AFAICT there is not really any downside to this in-practice. We get to:

  • Shrink terminators by 1-word per-terminator(e.g. every branch)
  • Remove a lot of unnecessary complexity from core data structures
  • Enable new use cases

I mostly just wanted to make sure that people were aware of this before submitting a bunch of changes to the core IR.

4

Awesome. I think it shrinks it by one word per successor, which is even better :). In any case, great work!

5

This makes total sense. Thanks, River!