feat: add control flow branch interface - #1237
Conversation
math-fehr
left a comment
There was a problem hiding this comment.
Nice! Can you move getSuccessorOperands to HasOpInfo though? And just return an Array ValuePtr?
|
looks nice overall but I definitely agree that the dialect-specific parts of this need to be pushed into the dialect files |
|
also use a WfIRContext instead of a raw context? |
None of the other interfaces I've looked at does this
Will do, I'll move things around |
1769770 to
5f81a28
Compare
math-fehr
left a comment
There was a problem hiding this comment.
Okay nice, I just have some comments on the notation then (since you are removing the checks that are not necessary). And then that should be good!
5f81a28 to
096f5e1
Compare
math-fehr
left a comment
There was a problem hiding this comment.
Nice! Sorry I just have one last request, but I'll merge it right after!
|
Thanks! |
Implemented a very simple interface for branch operations. I skipped a lot of extra stuff (such as supporting branch ops that produce operands internally by the branch op - something VeIR has none of right now) that's in here: https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Interfaces/ControlFlowInterfaces.h but it's a start. This will help #854 which would use this interface. The reason
SuccessorOperandsis a one field structure is to later have additional fields like the number of produced operands like I mentioned. Right now it'd be unused so I left it as is.