[PATCH] D23723: [SLP] Avoid signed integer overflow

[Matthew Simpson via llvm-commits]

mssimpso added a comment.

In https://reviews.llvm.org/D23723#521236, @mkuper wrote:

> Using SaturatingAdd here seems right, but:
>
> a) I'd prefer it if the test did check something beyond "don't crash".


Yeah, I agree. The difficulty now is that the total cost will be computed as INT_MAX (correctly), which will prevent vectorization in the first place. This really obfuscates the original problem. INT_MAX is set because of the depth < 3 check. The best thing to do might be to add a new command line option specifying the depth at which we declare the tree completely unprofitable to vectorize (instead of hard-coding it to 3). We can then raise the threshold in one run-line to avoid INT_MAX and re-enable vectorization. Another run-line can use the default threshold and ensure the cost doesn't wrap.

> b) It would be better it if one of the language lawyers looked at the SaturatingAdd implementation. In particular, I'm not sure casting an out-of-range unsigned to signed is well-defined.


Sure, I'll add more folks to the review.


https://reviews.llvm.org/D23723