On Wed, Aug 01, 2018 at 07:13:53AM -0500, Vlad Lazar wrote:
> On 31/07/18 22:48, James Greenhalgh wrote:
> > On Fri, Jul 20, 2018 at 04:37:34AM -0500, Vlad Lazar wrote:
> >> Hi,
> >>
> >> The patch adds implementations for the NEON intrinsics vabsd_s64 and
> >> vnegd_s64.
> >> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/docs/ihi0073/latest/arm-neon-intrinsics-reference-architecture-specification)
> >>
> >> Bootstrapped and regtested on aarch64-none-linux-gnu and there are no
> >> regressions.
> >>
> >> OK for trunk?
> >>
> >> +__extension__ extern __inline int64_t
> >> +__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> >> +vnegd_s64 (int64_t __a)
> >> +{
> >> + return -__a;
> >> +}
> >
> > Does this give the correct behaviour for the minimum value of int64_t? That
> > would be undefined behaviour in C, but well-defined under ACLE.
> >
> > Thanks,
> > James
> >
>
> Hi. Thanks for the review.
>
> For the minimum value of int64_t it behaves as the ACLE specifies:
> "The negative of the minimum (signed) value is itself."
What should happen in this testcase? The spoiler is below, but try to work out
what should happen and what goes wrong with your implementation.
int foo (int64_t x)
{
if (x < (int64_t) 0)
return vnegd_s64(x) < (int64_t) 0;
else
return 0;
}
int bar (void)
{
return foo (INT64_MIN);
}
Thanks,
James
-----
<spoiler!>
INT64_MIN < 0 should be true, so we should return vnegd_s64(INT64_MIN) < 0.
vnegd_s64(INT64_MIN) is identity, so the return value should be
INT64_MIN < 0; i.e. True.
This isn't what the compiler thinks... The compiler makes use of the fact
that -INT64_MIN is undefined behaviour in C, and doesn't need to be considered
as a special case. The if statement gives you a range reduction to [-INF, -1],
negating that gives you a range [1, INF], and [1, INF] is never less than 0,
so the compiler folds the function to return false. We have a mismatch in
semantics
