This is the mail archive of the
`gcc-patches@gcc.gnu.org`
mailing list for the GCC project.

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |

Other format: | [Raw text] |

*From*: Tamar Christina <Tamar dot Christina at arm dot com>*To*: Richard Biener <rguenther at suse dot de>*Cc*: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>, Jeff Law <law at redhat dot com>, "ian at airs dot com" <ian at airs dot com>, Joseph Myers <joseph at codesourcery dot com>*Date*: Tue, 2 Jul 2019 16:43:54 +0000*Subject*: Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)*References*: <20190625083126.GA19632@arm.com> <DB6PR0802MB230918BEA271728437E6CA57FFE30@DB6PR0802MB2309.eurprd08.prod.outlook.com> <alpine.LSU.2.20.1906251044350.10704@zhemvz.fhfr.qr> <20190702094115.GA22370@arm.com> <alpine.LSU.2.20.1907021214070.2976@zhemvz.fhfr.qr>

Hi All, Here's an updated patch with the changes processed from the previous review. I've bootstrapped and regtested on aarch64-none-linux-gnu and x86_64-pc-linux-gnu and no issues. Ok for trunk? Thanks, Tamar The 07/02/2019 11:20, Richard Biener wrote: > On Tue, 2 Jul 2019, Tamar Christina wrote: > > > Hi Richi, > > > > The 06/25/2019 10:02, Richard Biener wrote: > > > > > > This looks like a literal 1:1 translation plus merging with the > > > existing pattern around integers. You changed > > > (op:s@0 (convert@3 @1) (convert?@4 @2)) to > > > (op:s@0 (convert1?@3 @1) (convert2?@4 @2)) where this now also > > > matches if there are no inner conversions at all - was that a > > > desired change or did you merely want to catch when the first > > > operand is not a conversion but the second is, possibly only > > > for the RDIV_EXPR case? > > > > > > > Yes, the ? ? is for the RDIV case, I really only want (c a) `op` (c b), > > a `op` (c b) and (c a) `op` b. But didn't find a way to do this. > > One way would be to do > > (simplify > (convert (op:sc@0 (convert @1) (convert? @2))) > > but that doesn't work for RDIV. Using :C is tempting but you > do not get to know the original operand order which you of > course need. So I guess the way you do it is fine - you > could guard all of the code with a few types_match () checks > but I'm not sure it is worth the trouble. > > Richard. > > > The only thing I can think of that gets this is without overmatching is > > to either duplicate the code or move this back to a C helper function and > > call that from match.pd. But I was hoping to have it all in match.pd > > instead of hiding it away in a C call. > > > > Do you have a better way of doing it or a preference to an approach? > > > > > +(for op (plus minus mult rdiv) > > > + (simplify > > > + (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2))) > > > + (with { tree arg0 = strip_float_extensions (@1); > > > + tree arg1 = strip_float_extensions (@2); > > > + tree itype = TREE_TYPE (@0); > > > > > > you now unconditionally call strip_float_extensions on each operand > > > even for the integer case, please sink stuff only used in one > > > case arm. I guess keeping the integer case first via > > > > > > > Done, Initially didn't think it would be an issue since I don't use the value it > > creates in the integer case. But I re-ordered it. > > > > > should work (with the 'with' being in the ifs else position). > > > > > > + (if (code == REAL_TYPE) > > > + /* Ignore the conversion if we don't need to store intermediate > > > + results and neither type is a decimal float. */ > > > + (if (!(flag_float_store > > > + || DECIMAL_FLOAT_TYPE_P (type) > > > + || DECIMAL_FLOAT_TYPE_P (itype)) > > > + && types_match (ty1, ty2)) > > > + (convert (op (convert:ty1 @1) (convert:ty2 @2))))) > > > > > > this looks prone to the same recursion issue you described above. > > > > It's to break the recursion when you don't match anything. Indeed don't need it if > > I change the match condition above. > > Thanks, > > Tamar > > > > > > 'code' is used exactly once, using SCALAR_FLOAT_TYPE_P (itype) > > > in the above test would be clearer. Also both ifs can be combined. > > > The snipped above also doesn't appear in the convert.c code you > > > remove and the original one is > > > > > > switch (TREE_CODE (TREE_TYPE (expr))) > > > { > > > case REAL_TYPE: > > > /* Ignore the conversion if we don't need to store intermediate > > > results and neither type is a decimal float. */ > > > return build1_loc (loc, > > > (flag_float_store > > > || DECIMAL_FLOAT_TYPE_P (type) > > > || DECIMAL_FLOAT_TYPE_P (itype)) > > > ? CONVERT_EXPR : NOP_EXPR, type, expr); > > > > > > which as far as I can see doesn't do anything besides > > > exchanging CONVERT_EXPR for NOP_EXPR which is a noop to the IL. > > > So it appears this shouldn't be moved to match.pd at all? > > > It's also not a 1:1 move since you are changing 'expr'. > > > > > > Thanks, > > > Richard. > > > > > > > > Thanks, > > > > > Tamar > > > > > > > > > > Concretely it makes both these cases behave the same > > > > > > > > > > float e = (float)a * (float)b; > > > > > *c = (_Float16)e; > > > > > > > > > > and > > > > > > > > > > *c = (_Float16)((float)a * (float)b); > > > > > > > > > > Thanks, > > > > > Tamar > > > > > > > > > > gcc/ChangeLog: > > > > > > > > > > 2019-06-25 Tamar Christina <tamar.christina@arm.com> > > > > > > > > > > * convert.c (convert_to_real_1): Move part of conversion code... > > > > > * match.pd: ...To here. > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > 2019-06-25 Tamar Christina <tamar.christina@arm.com> > > > > > > > > > > * gcc.dg/type-convert-var.c: New test. > > > > > > > > > > -- > > > > > > > > > > -- > > > Richard Biener <rguenther@suse.de> > > > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; > > > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg) > > > > > > -- > Richard Biener <rguenther@suse.de> > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg) --

diff --git a/gcc/convert.c b/gcc/convert.c index a8f2bd049ba0cd83865ba0a5f7d74f9cdbad0d09..7f0d933f4d9e29719acb27eb1b32a9e540d93073 100644 --- a/gcc/convert.c +++ b/gcc/convert.c @@ -298,92 +298,6 @@ convert_to_real_1 (tree type, tree expr, bool fold_p) return build1 (TREE_CODE (expr), type, arg); } break; - /* Convert (outertype)((innertype0)a+(innertype1)b) - into ((newtype)a+(newtype)b) where newtype - is the widest mode from all of these. */ - case PLUS_EXPR: - case MINUS_EXPR: - case MULT_EXPR: - case RDIV_EXPR: - { - tree arg0 = strip_float_extensions (TREE_OPERAND (expr, 0)); - tree arg1 = strip_float_extensions (TREE_OPERAND (expr, 1)); - - if (FLOAT_TYPE_P (TREE_TYPE (arg0)) - && FLOAT_TYPE_P (TREE_TYPE (arg1)) - && DECIMAL_FLOAT_TYPE_P (itype) == DECIMAL_FLOAT_TYPE_P (type)) - { - tree newtype = type; - - if (TYPE_MODE (TREE_TYPE (arg0)) == SDmode - || TYPE_MODE (TREE_TYPE (arg1)) == SDmode - || TYPE_MODE (type) == SDmode) - newtype = dfloat32_type_node; - if (TYPE_MODE (TREE_TYPE (arg0)) == DDmode - || TYPE_MODE (TREE_TYPE (arg1)) == DDmode - || TYPE_MODE (type) == DDmode) - newtype = dfloat64_type_node; - if (TYPE_MODE (TREE_TYPE (arg0)) == TDmode - || TYPE_MODE (TREE_TYPE (arg1)) == TDmode - || TYPE_MODE (type) == TDmode) - newtype = dfloat128_type_node; - if (newtype == dfloat32_type_node - || newtype == dfloat64_type_node - || newtype == dfloat128_type_node) - { - expr = build2 (TREE_CODE (expr), newtype, - convert_to_real_1 (newtype, arg0, - fold_p), - convert_to_real_1 (newtype, arg1, - fold_p)); - if (newtype == type) - return expr; - break; - } - - if (TYPE_PRECISION (TREE_TYPE (arg0)) > TYPE_PRECISION (newtype)) - newtype = TREE_TYPE (arg0); - if (TYPE_PRECISION (TREE_TYPE (arg1)) > TYPE_PRECISION (newtype)) - newtype = TREE_TYPE (arg1); - /* Sometimes this transformation is safe (cannot - change results through affecting double rounding - cases) and sometimes it is not. If NEWTYPE is - wider than TYPE, e.g. (float)((long double)double - + (long double)double) converted to - (float)(double + double), the transformation is - unsafe regardless of the details of the types - involved; double rounding can arise if the result - of NEWTYPE arithmetic is a NEWTYPE value half way - between two representable TYPE values but the - exact value is sufficiently different (in the - right direction) for this difference to be - visible in ITYPE arithmetic. If NEWTYPE is the - same as TYPE, however, the transformation may be - safe depending on the types involved: it is safe - if the ITYPE has strictly more than twice as many - mantissa bits as TYPE, can represent infinities - and NaNs if the TYPE can, and has sufficient - exponent range for the product or ratio of two - values representable in the TYPE to be within the - range of normal values of ITYPE. */ - if (TYPE_PRECISION (newtype) < TYPE_PRECISION (itype) - && (flag_unsafe_math_optimizations - || (TYPE_PRECISION (newtype) == TYPE_PRECISION (type) - && real_can_shorten_arithmetic (TYPE_MODE (itype), - TYPE_MODE (type)) - && !excess_precision_type (newtype)))) - { - expr = build2 (TREE_CODE (expr), newtype, - convert_to_real_1 (newtype, arg0, - fold_p), - convert_to_real_1 (newtype, arg1, - fold_p)); - if (newtype == type) - return expr; - } - } - } - break; default: break; } diff --git a/gcc/match.pd b/gcc/match.pd index f8e35e96d22036bb0b96fbdbe2c7a346f4695067..66a1ad385ffff4456c87a8891ab78c437fefc64f 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -4937,37 +4937,106 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) the C/C++ front-ends by shorten_binary_op and shorten_compare. Long term we want to move all that code out of the front-ends into here. */ -/* If we have a narrowing conversion of an arithmetic operation where - both operands are widening conversions from the same type as the outer - narrowing conversion. Then convert the innermost operands to a suitable - unsigned type (to avoid introducing undefined behavior), perform the - operation and convert the result to the desired type. */ -(for op (plus minus) - (simplify - (convert (op:s (convert@2 @0) (convert?@3 @1))) - (if (INTEGRAL_TYPE_P (type) - /* We check for type compatibility between @0 and @1 below, - so there's no need to check that @1/@3 are integral types. */ - && INTEGRAL_TYPE_P (TREE_TYPE (@0)) - && INTEGRAL_TYPE_P (TREE_TYPE (@2)) - /* The precision of the type of each operand must match the - precision of the mode of each operand, similarly for the - result. */ - && type_has_mode_precision_p (TREE_TYPE (@0)) - && type_has_mode_precision_p (TREE_TYPE (@1)) - && type_has_mode_precision_p (type) - /* The inner conversion must be a widening conversion. */ - && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0)) - && types_match (@0, type) - && (types_match (@0, @1) - /* Or the second operand is const integer or converted const - integer from valueize. */ - || TREE_CODE (@1) == INTEGER_CST)) - (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))) - (op @0 (convert @1)) - (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } - (convert (op (convert:utype @0) - (convert:utype @1)))))))) +/* Convert (outertype)((innertype0)a+(innertype1)b) + into ((newtype)a+(newtype)b) where newtype + is the widest mode from all of these. */ +(for op (plus minus mult rdiv) + (simplify + (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2))) + /* If we have a narrowing conversion of an arithmetic operation where + both operands are widening conversions from the same type as the outer + narrowing conversion. Then convert the innermost operands to a + suitable unsigned type (to avoid introducing undefined behavior), + perform the operation and convert the result to the desired type. */ + (if (INTEGRAL_TYPE_P (type) + && op != MULT_EXPR + && op != RDIV_EXPR + /* We check for type compatibility between @0 and @1 below, + so there's no need to check that @2/@4 are integral types. */ + && INTEGRAL_TYPE_P (TREE_TYPE (@1)) + && INTEGRAL_TYPE_P (TREE_TYPE (@3)) + /* The precision of the type of each operand must match the + precision of the mode of each operand, similarly for the + result. */ + && type_has_mode_precision_p (TREE_TYPE (@1)) + && type_has_mode_precision_p (TREE_TYPE (@2)) + && type_has_mode_precision_p (type) + /* The inner conversion must be a widening conversion. */ + && TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (TREE_TYPE (@1)) + && types_match (@1, type) + && (types_match (@1, @2) + /* Or the second operand is const integer or converted const + integer from valueize. */ + || TREE_CODE (@2) == INTEGER_CST)) + (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1))) + (op @1 (convert @2)) + (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); } + (convert (op (convert:utype @1) + (convert:utype @2))))) + (with { tree arg0 = strip_float_extensions (@1); + tree arg1 = strip_float_extensions (@2); + tree itype = TREE_TYPE (@0); + tree ty1 = TREE_TYPE (arg0); + tree ty2 = TREE_TYPE (arg1); + enum tree_code code = TREE_CODE (itype); } + (if (FLOAT_TYPE_P (ty1) + && FLOAT_TYPE_P (ty2) + && FLOAT_TYPE_P (type) + && DECIMAL_FLOAT_TYPE_P (itype) == DECIMAL_FLOAT_TYPE_P (type)) + (with { tree newtype = type; + if (TYPE_MODE (ty1) == SDmode + || TYPE_MODE (ty2) == SDmode + || TYPE_MODE (type) == SDmode) + newtype = dfloat32_type_node; + if (TYPE_MODE (ty1) == DDmode + || TYPE_MODE (ty2) == DDmode + || TYPE_MODE (type) == DDmode) + newtype = dfloat64_type_node; + if (TYPE_MODE (ty1) == TDmode + || TYPE_MODE (ty2) == TDmode + || TYPE_MODE (type) == TDmode) + newtype = dfloat128_type_node; } + (if ((newtype == dfloat32_type_node + || newtype == dfloat64_type_node + || newtype == dfloat128_type_node) + && newtype == type) + (convert:newtype (op (convert:newtype @1) (convert:newtype @2))) + (with { if (TYPE_PRECISION (ty1) > TYPE_PRECISION (newtype)) + newtype = ty1; + if (TYPE_PRECISION (ty2) > TYPE_PRECISION (newtype)) + newtype = ty2; } + /* Sometimes this transformation is safe (cannot + change results through affecting double rounding + cases) and sometimes it is not. If NEWTYPE is + wider than TYPE, e.g. (float)((long double)double + + (long double)double) converted to + (float)(double + double), the transformation is + unsafe regardless of the details of the types + involved; double rounding can arise if the result + of NEWTYPE arithmetic is a NEWTYPE value half way + between two representable TYPE values but the + exact value is sufficiently different (in the + right direction) for this difference to be + visible in ITYPE arithmetic. If NEWTYPE is the + same as TYPE, however, the transformation may be + safe depending on the types involved: it is safe + if the ITYPE has strictly more than twice as many + mantissa bits as TYPE, can represent infinities + and NaNs if the TYPE can, and has sufficient + exponent range for the product or ratio of two + values representable in the TYPE to be within the + range of normal values of ITYPE. */ + (if (TYPE_PRECISION (newtype) < TYPE_PRECISION (itype) + && (flag_unsafe_math_optimizations + || (TYPE_PRECISION (newtype) == TYPE_PRECISION (type) + && real_can_shorten_arithmetic (TYPE_MODE (itype), + TYPE_MODE (type)) + && !excess_precision_type (newtype)))) + (convert:newtype (op (convert:newtype @1) + (convert:newtype @2))) + )))) ) + ) +))) /* This is another case of narrowing, specifically when there's an outer BIT_AND_EXPR which masks off bits outside the type of the innermost diff --git a/gcc/testsuite/gcc.dg/type-convert-var.c b/gcc/testsuite/gcc.dg/type-convert-var.c new file mode 100644 index 0000000000000000000000000000000000000000..88d74e2a49d7123515b87ff64a18bd9b306d57e9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/type-convert-var.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-O1 -fdump-tree-optimized" } */ +void foo (float a, float b, float *c) +{ + double e = (double)a * (double)b; + *c = (float)e; +} + +/* { dg-final { scan-tree-dump-not {double} "optimized" } } */

**Follow-Ups**:**Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)***From:*Richard Biener

**[PATCH] Fix up gcc.dg/type-convert-var.c testcase (was Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch))***From:*Jakub Jelinek

**Fix up -fexcess-precision handling in LTO (was Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch))***From:*Jakub Jelinek

**References**:

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |