dialects: (scf) `scf.WhileOp` crashes with ValueError when regions contain multiple blocks

[wulu007]

Description

The current implementation of WhileOp in the SCF dialect fails when its regions contain more than one block. This is because it accesses the entry block arguments using the .block property of the Region class, which raises a ValueError if the region contains multiple blocks.

In MLIR, scf.while regions are allowed to have multiple blocks (forming a Control Flow Graph). The initialization arguments and results should only be mapped to the entry block of the respective regions.

https://github.com/llvm/llvm-project/blob/47fb7cd732ffc666d2865e1a187db24f8e09ab25/mlir/lib/Dialect/SCF/IR/SCF.cpp#L3328

Expected Behavior

WhileOp should be able to print and verify correctly even if the before or after regions contain multiple blocks.

[superlopuh]

Hi, this is not expected behavior. The expected behavior is that the op does not verify if there are multiple blocks, and we are missing this verification check. The design of the scf dialect is to express "structured" control flow, which is based on regions. It also contains an op explicitly as an "escape hatch" to allow multiple blocks in the region. Can you please open a new PR adding the verification? Ideally we should have a similar error to what MLIR will raise if you pass it the test file from your PR.

[superlopuh]

Also, feel free to write on the Zulip or open issues first when you spot a bug, that way we can have a conversation about the implementation plan before investing time in code.

[wulu007]

Hi, this is not expected behavior. The expected behavior is that the op does not verify if there are multiple blocks, and we are missing this verification check. The design of the scf dialect is to express "structured" control flow, which is based on regions. It also contains an op explicitly as an "escape hatch" to allow multiple blocks in the region. Can you please open a new PR adding the verification? Ideally we should have a similar error to what MLIR will raise if you pass it the test file from your PR.

In xDSL, region_def() defaults to a multi-block region unless single_block is explicitly passed. Since WhileOp was defined using the generic region_def() rather than region_def("single_block"), it led me to believe that the operation was intended to be compatible with multiple blocks, similar to scf.execute_region.

Maybe you can do that?

@irdl_op_definition
class WhileOp(IRDLOperation):
    name = "scf.while"
    arguments = var_operand_def()

    res = var_result_def()
    before_region = region_def('single_block')
    after_region = region_def('single_block')

[superlopuh]

This would be a great change to make, together with two lit tests, one for each of the regions. Would you have time to make this change?