[llvm] [ARM][ConstantIslands] Correct MinNoSplitDisp calculation (PR #114590)

via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 11 12:29:39 PST 2024


pzhengqc wrote:

> Adding some Arm folks, as constant island isn't as trivial as it sounds. Your description makes sense and the code could be interpreted as "someone forgot to subtract the start offset", but I want to make sure this is what we think it is.
> 
> Also, needs tests. I imagine a "large enough" function IR to trigger the split but a small enough BB IR to not be necessary, and show that there are no errors or crashes.

Thanks for the review, @rengolin! I think this hasn't been caught for so long because it's actually hard to trigger the check `MinNoSplitDisp > U.getMaxDisp() / 2`. The check is only performed when `CloserWater` is true which means the pass has not converged after 15 iterations with the default `arm-constant-island-max-iteration` being 30. In reality, it rarely seems to take more than 3 iterations for the pass to converge.

I've also added a test case to show that, with this fix, a small BB is not split even if it's far away from the beginning of the function.

https://github.com/llvm/llvm-project/pull/114590


More information about the llvm-commits mailing list