We do not try to move the instructions and split the block till we
know the blocks can be split, i.e. BCE-cmp-insts can be separated from
Thanks a lot for the patch.
Typo: extra "this".
Typo: side effects.
This will unconditionally split the block even if any subsequent condition for merging fails. This has following issues:
To fix this, you could add the block to the chain with a tag and do the actual splitting in simplify().
For the first point: What about adding a test with a single splittable block and check that the pass introduces no changes ?
Actually tryToSplitBCECmpBlock will not move instructions around if it can not split the block. i.e. it first tests whether all non-bce-cmp instructions can be separated from bce-cmp instructions.
The way I think about the problem is that we have a chain and within the chain there may be blocks that do work other than the BCE compare which stops us from collapsing the chain into a memcmp. This block can be the first block of the chain or in middle of the chain.
In case its the first block in the chain, we can choose to split it (or discard it in case we can not split it), which is what we do with this patch. NOTE: the way split is done will not affect what is recorded in BCECmpBlock. More specifically, a new block is created and all the non-bce-cmp instructions are moved to the new BB. The old BB stays what it is as far as bce-cmp instructions are concerned.
In case its in middle of the chain, its a bit more complicated, we need to terminate the chain, process what has been collected and restart anew (This has not been implemented, but i can see how we can implement this even with this patch. i.e. we can break out of the loop and process what has been collected. Then try to rerun the chain collecting/collapsing process again on the modified IR).
Sure, I understand what this fixes and I think it's worth fixing.