[PATCH] D61084: [ConstantRange] Add abs() support

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D61084#1477695 <https://reviews.llvm.org/D61084#1477695>, @lebedev.ri wrote:

> The test certainly works, because we are only checking 4-bit values, while `APInt` is 64-bit or more.
>  What happens for `ConstantRange`'s of larger width?


The bit width doesn't make a difference here: If the signed min //for the particular bitwidth// is in the original range, it will also be in the result range.

> Maybe you should return `ConstantRange` with bitwidth larger by one if `INT_MIN` is included?

I don't think this would be particularly useful, at least in the context where this is intended to be used: The abs() implementation here models the semantics of SPF_ABS, which maps signed min onto signed min, and it's important to preserve that edge-case behavior.

We have similar code for handling abs of signed min in ValueTracking: https://github.com/llvm-mirror/llvm/blob/4d4155f08ea87c6909ddd6db4610c3d6c8034983/lib/Analysis/ValueTracking.cpp#L5669-L5679


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61084/new/

https://reviews.llvm.org/D61084