On Fri, Jan 8, 2016 at 11:24 PM, Joseph Myers <jos...@codesourcery.com> wrote: > On Fri, 8 Jan 2016, Matthew Wahab wrote: > >> Hello, >> >> The C/C++ front-ends apply type conversions to expressions using ABS >> with integral arguments of type smaller than int. This means that, for >> short x, ABS(x) becomes something like (short)ABS((int)x)). When the >> argument is the result of memory access, this restricts the vectorizer >> apparently because the alignment of the operation doesn't match the >> alignment of the memory access. This causes two failures in >> gcc.target/aarch64/vec-abs-compile.c where the expected abs instructions >> aren't generated. >> >> This patch adds a case to convert_to_integer_1 to push the integer type >> conversions applied to an ABS expression into the argument, when it is >> safe to do so. This fixes the failing test cases. > > Note that, for example, (int) labs (INT_MIN) is well-defined if long is > wider than int (in GNU C, where conversion of out-of-range values to > signed types is well-defined). But abs (INT_MIN) is undefined. That is, > such transformations can never be safe unless the minimum value of the > type of the ABS_EXPR argument is not a possible value of the contained > expression. > > What's your definition of safe? I'd expect a patch like this to contain > lots of testcases verifying that the transformation occurs in cases where > it's OK, but that (int) labs (int_var) does not end up with an ABS_EXPR on > type int (for example) (in the absence of a guarantee that said ABS_EXPR > will return INT_MIN for an argument of INT_MIN - which might be the case > for -fwrapv, but can't be otherwise because *_nonnegative*_p assume > ABS_EXPR always produces a nonnegative result).
Also please don't add to convert_* and friends but instead amend match.pd with appropriate rules. Richard. > -- > Joseph S. Myers > jos...@codesourcery.com
