Message ID | CAFiYyc2u-zSP_DErk73ZEUXOgLfeT3NGa+fiA6uu5JxWFZjurg@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Thu, 30 Apr 2015, Richard Biener wrote: > I have in my local dev tree (so completely untested...) > > @@ -1040,31 +1052,22 @@ (define_operator_list CBRT BUILT_IN_CBRT > operation and convert the result to the desired type. */ > (for op (plus minus) > (simplify > - (convert (op (convert@2 @0) (convert@3 @1))) > + (convert (op:c@4 (convert@2 @0) (convert?@3 @1))) I believe the :c here requires extra code further down, so we don't turn a-b into b-a. > (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)) > + && INTEGRAL_TYPE_P (TREE_TYPE (@4)) > /* The precision of the type of each operand must match the > precision of the mode of each operand, similarly for the > result. */ > && (TYPE_PRECISION (TREE_TYPE (@0)) > == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (@0)))) > - && (TYPE_PRECISION (TREE_TYPE (@1)) > - == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (@1)))) > - && TYPE_PRECISION (type) == GET_MODE_PRECISION (TYPE_MODE (type)) > /* The inner conversion must be a widening conversion. */ > && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0)) > - && ((GENERIC > - && (TYPE_MAIN_VARIANT (TREE_TYPE (@0)) > - == TYPE_MAIN_VARIANT (TREE_TYPE (@1))) > - && (TYPE_MAIN_VARIANT (TREE_TYPE (@0)) > - == TYPE_MAIN_VARIANT (type))) > - || (GIMPLE > - && types_compatible_p (TREE_TYPE (@0), TREE_TYPE (@1)) > - && types_compatible_p (TREE_TYPE (@0), type)))) > + /* The final precision should match that of operand @0. */ > + && TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)) > + /* Make sure the wide operation is dead after the transform. */ > + && (TREE_CODE (@4) != SSA_NAME || has_single_use (@4))) > (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))) > - (convert (op @0 @1))) > + (convert (op @0 (convert @1)))) > (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } > (convert (op (convert:utype @0) (convert:utype @1)))))))
On Thu, Apr 30, 2015 at 12:53 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Thu, 30 Apr 2015, Richard Biener wrote: > >> I have in my local dev tree (so completely untested...) >> >> @@ -1040,31 +1052,22 @@ (define_operator_list CBRT BUILT_IN_CBRT >> operation and convert the result to the desired type. */ >> (for op (plus minus) >> (simplify >> - (convert (op (convert@2 @0) (convert@3 @1))) >> + (convert (op:c@4 (convert@2 @0) (convert?@3 @1))) > > > I believe the :c here requires extra code further down, so we don't turn a-b > into b-a. Indeed. I've added :c only for minus as 5 - x can't be canonicalized to move the constant to 2nd position which is always possible for plus. Might be cleaner to add a separate pattern for that case. Richard. > >> (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)) >> + && INTEGRAL_TYPE_P (TREE_TYPE (@4)) >> /* The precision of the type of each operand must match the >> precision of the mode of each operand, similarly for the >> result. */ >> && (TYPE_PRECISION (TREE_TYPE (@0)) >> == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (@0)))) >> - && (TYPE_PRECISION (TREE_TYPE (@1)) >> - == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (@1)))) >> - && TYPE_PRECISION (type) == GET_MODE_PRECISION (TYPE_MODE (type)) >> /* The inner conversion must be a widening conversion. */ >> && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE >> (@0)) >> - && ((GENERIC >> - && (TYPE_MAIN_VARIANT (TREE_TYPE (@0)) >> - == TYPE_MAIN_VARIANT (TREE_TYPE (@1))) >> - && (TYPE_MAIN_VARIANT (TREE_TYPE (@0)) >> - == TYPE_MAIN_VARIANT (type))) >> - || (GIMPLE >> - && types_compatible_p (TREE_TYPE (@0), TREE_TYPE (@1)) >> - && types_compatible_p (TREE_TYPE (@0), type)))) >> + /* The final precision should match that of operand @0. */ >> + && TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)) >> + /* Make sure the wide operation is dead after the transform. */ >> + && (TREE_CODE (@4) != SSA_NAME || has_single_use (@4))) >> (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))) >> - (convert (op @0 @1))) >> + (convert (op @0 (convert @1)))) >> (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } >> (convert (op (convert:utype @0) (convert:utype @1))))))) > > > -- > Marc Glisse
On 04/30/2015 03:00 AM, Richard Biener wrote: > > Without looking too close at this patch I'll note that we might want to > improve the previous one first to also handle a constant 2nd operand > for the operation (your new one also misses that). Yea, I think you mentioned in that in the 47477 BZ as well. If you've got testcases, pass them along so that we can build testcases around those forms as well. > and it was noticed multiple times that the type comparison boiler-plate > needs some helper function. Like Yes. It's on the TODO list. There's certainly more follow-ups in the pipeline. If we want to factor out the boiler-plate now, that works for me. > > And if you'd like to lend helping hands to adding patterns then transitioning > patterns from fold-const.c to match.pd is more appreciated than inventing > new ones ;) The next round of work is much more likely to be reimplementing the operand shortening code shared between the C/C++ front-ends in match.pd and removal of the C/C++ operand shortening code. This patch didn't fit into that work terribly well and seemed self-contained enough to go forward now rather than waiting. Jeff
On 04/30/2015 05:07 AM, Richard Biener wrote: > On Thu, Apr 30, 2015 at 12:53 PM, Marc Glisse <marc.glisse@inria.fr> wrote: >> On Thu, 30 Apr 2015, Richard Biener wrote: >> >>> I have in my local dev tree (so completely untested...) >>> >>> @@ -1040,31 +1052,22 @@ (define_operator_list CBRT BUILT_IN_CBRT >>> operation and convert the result to the desired type. */ >>> (for op (plus minus) >>> (simplify >>> - (convert (op (convert@2 @0) (convert@3 @1))) >>> + (convert (op:c@4 (convert@2 @0) (convert?@3 @1))) >> >> >> I believe the :c here requires extra code further down, so we don't turn a-b >> into b-a. > > Indeed. I've added :c only for minus as 5 - x can't be canonicalized to > move the constant to 2nd position which is always possible for plus. > > Might be cleaner to add a separate pattern for that case. FWIW, this is the patch that's attached to 65084 (not 47477 as I initially reported). It's supposed to address one or more of the example loops in that testcase. I'd like to tackle it as a follow-up. jeff
Index: gimple-match-head.c =================================================================== --- gimple-match-head.c (revision 222375) +++ gimple-match-head.c (working copy) @@ -861,3 +861,8 @@ return op; } +inline bool +types_match (tree t1, tree t2) +{ + return types_compatible_p (t1, t2); +} Index: generic-match-head.c =================================================================== --- generic-match-head.c (revision 222375) +++ generic-match-head.c (working copy) @@ -70,4 +70,8 @@ #include "dumpfile.h" #include "generic-match.h" - +inline bool +types_match (tree t1, tree t2) +{ + return TYPE_MAIN_VARIANT (t1) == TYPE_MAIN_VARIANT (t2); +}