The RegionBranchOpInterface had a few fundamental issues caused by the API design of getSuccessorRegions.
It always required passing values for the operands parameter. This is problematic as the operands parameter actually changes meaning depending on which predecessor index is referring to. If coming from a region, you'd have to find a RegionBranchTerminatorOpInterface in that region, get its operand count, and then create a SmallVector of that size.
This is not only inconvenient, but also error-prone, which has lead to a bug in the implementation of a previously existing getSuccessorRegions overload.
Additionally, this made the method dual-use, trying to serve two different use-cases: 1) Trying to determine possible control flow edges between regions and 2) Trying to determine the region being branched to based on constant operands.
This patch fixes these issues by changing the interface methods and adding new ones:
- The operands argument of getSuccessorRegions has been removed. The method is now only responsible for returning possible control flow edges between regions.
- An optional getEntrySuccessorRegions method has been added. This is used to determine which regions are branched to from the parent op based on constant operands of the parent op. By default, it calls getSuccessorRegions. This is analogous to getSuccessorForOperands from BranchOpInterface.
- Add getSuccessorRegions to RegionBranchTerminatorOpInterface. This is used to get the possible successors of the terminator based on constant operands. By default, it calls the containing RegionBranchOpInterfaces getSuccessorRegions method.
- getSuccessorEntryOperands was renamed to getEntrySuccessorOperands for consistency.
Perfect. This API was definitely not ideal. The new method makes things abundantly clear.