Re: [PATCH 2/9][GCC][AArch64][middle-end] Add rules to strip away unneeded type casts in expressions

[Joseph Myers <joseph at codesourcery dot com>]

On Sun, 11 Nov 2018, Tamar Christina wrote:

> This patch adds a match.pd rule for stripping away the type converts 
> when you're converting to a type that has twice the precision of the 
> current type in the same class, doing a simple math operation on it and 
> converting back to the smaller type.

What types exactly is this meant to apply to?  Floating-point?  Integer?  
Mixtures of those?  (I'm guessing not mixtures, because those would be 
something other than "convert" here.)

For integer types, it's not safe, in that if e.g. F is int and X is 
unsigned long long, you're changing from defined overflow to undefined 
overflow.

For floating-point types, using TYPE_PRECISION is suspect (it's not 
wonderfully clear what it means, but it's not the number of significand 
bits) - you need to look at the actual properties of the real format of 
the machine modes in question.

Specifically, see convert.c:convert_to_real_1, the long comment starting 
"Sometimes this transformation is safe (cannot change results through 
affecting double rounding cases) and sometimes it is not.", and the 
associated code calling real_can_shorten_arithmetic.  I think that code in 
convert.c ought to apply to your case of half precision converted to float 
for arithmetic and then converted back to half precision afterwards.  (In 
the case where the excess precision configuration - which depends on 
TARGET_FP_F16INST for AArch64 - says to do arithmetic directly on half 
precision, anyway.)

Now, there are still issues in that convert.c code in the decimal 
floating-point case (see bug 40503).  And I think match.pd is a much 
better place for this sort of thing than convert.c (and 
c-family/c-common.c:shorten_binary_op in the integer case).  But for this 
specific case of binary floating-point conversions, I think the logic in 
convert.c is what should be followed (but moved to match.pd if possible).

This patch is also lacking a testcase, which might show why the existing 
logic in convert.c isn't being applied in whatever case you're concerned 
with.

-- 
Joseph S. Myers
joseph@codesourcery.com