core: add arg_types param to Block.split_before

[superlopuh]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9a268bd) 89.50% compared to head (0333a59) 89.49%.
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1944      +/-   ##
==========================================
- Coverage   89.50%   89.49%   -0.01%     
==========================================
  Files         279      279              
  Lines       34367    34387      +20     
  Branches     5094     5097       +3     
==========================================
+ Hits        30759    30776      +17     
- Misses       2873     2875       +2     
- Partials      735      736       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[math-fehr]

👍

[webmiche]

I guess block arguments are generally a bit of a headache for splitting blocks. E.g., what happens now if a moved op actually has a block argument of the original block as its operand? The IR will just be ill-formed, right? Maybe, we should try to catch this case in a test and make sure that IR is detected to be ill-formed.

Also, I guess you could also infer which block arguments the new block needs by looking at its def-use chain and seeing what is alive at the splitting point. But I guess that would kind of be overengineering at this point.

I am fine with the current solution, if we add the expected failure test I mentioned above and maybe a warning that this can be a problem. I guess it would be interesting at some point to look into a principled way of handling this in the splitting method, but for the moment, we can push the burden to the user.
(@superlopuh @math-fehr)

[superlopuh]

Yeah there are helpers to add here, potentially, but I think that the assumption is that most often the newly created block will be dominated by the original, so all the values are in scope, no argument magic necessary. Let's add helpers when we come across a use-case where that's not true.

[webmiche]

What do you think about the testcase I mentioned? You want me to open an issue, right? 😜