ARROW-12685: [C++][Compute] Add unary absolute value kernel

[edponce]

This PR adds the arithmetic absolute value kernels to the compute layer.

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - How to contribute patches

[github-actions]

https://issues.apache.org/jira/browse/ARROW-12685

[edponce]

I named the compute function as AbsoluteValue and kernels as "absolute_value" but this feels like too long a name. Convention across other libraries is "abs" but Arrow's compute kernels tend to follow the longer format and "absolute" seems ambiguous. @bkietz please advise

[edponce]

@cyb70289 I noticed that clang removes the check of if (result < 0) because it seems that during the analysis it has a rule that the result of std::abs() is never negative, but this is not true if the input is std::numeric_limits<T>::min(). In other words, it does not considers the overflow case, although the value returned by std::abs() is negative and correct. See https://godbolt.org/z/7bsM4oTMG to observe this effect. I also tested the godbolt example using unknown values at compile-time and observe the same behavior. This behavior is observed across compilers. My conclusion is to follow the form of abs_2(), that is, if (arg == std::numeric_limits<T>::min()).

[cyb70289]

Thanks @edponce for deep investigation.

From std::abs() document, it is UB if input is numeic_limit::min(). So compiler can do whatever optimization that doesn't violate the standard.

Computes the absolute value of an integer number. The behavior is undefined if the result cannot be represented by the return type.

For checked abs, you can check arg == min() before calling std::abs().
For non-checked abs, thought compiler generated code is safe (no UB), I think it's better to go back to return v < 0 ? (~(unsigned)v + 1) : v; approach. Though the assembly code is exactly the same, compiler may decide to change behaviour in the future.

[edponce]

For such a simple kernel we found:

1. We need to handle overflow case manually because the standard states it is undefined behavior
2. Compilers can prune checks that test for negative value the result of an absolute value operation. The purpose of such check is to detect overflow. For example,

result = std::abs(arg);
if (result < 0) {
ERROR(...);
}
return result;

becomes result = std::abs(arg). The solution is to first check if input is a value that will overflow

if (arg == std::numeric_limits<T>::min()) {
ERROR(...);
return arg;
}
result = std::abs(arg);

3. Tests performing checks for special cases such as -0.0 == 0.0 should also check the sign bit, std::signbit(val).

[cyb70289]

Also, would you rebase to fix some CI failures? #10310

[cyb70289]

LGTM, +1

[cyb70289]

@pitrou, do you have other comments to this PR?