On Thu, Nov 10, 2016 at 9:52 PM, Dominik Vogt <[email protected]> wrote:
> On Wed, Nov 09, 2016 at 03:46:38PM +0100, Richard Biener wrote:
>> On Wed, Nov 9, 2016 at 3:30 PM, Dominik Vogt <[email protected]> wrote:
>> > Something like the attached patch? Robin and me have spent quite
>> > some time to figure out the new pattern. Two questions:
>> >
>> > 1) In the match expression you cannot just use SSA_NAME@0 because
>> > then the "case SSA_NAME:" is added to a switch for another
>> > pattern that already has that label. Thus we made that "proxy"
>> > predicate "ssa_name_p" that forces the code for the new pattern
>> > out of the old switch and into a separate code block. We
>> > couldn't figure out whether this joining of case labels is a
>> > feature in the matching language. So, is this the right way to
>> > deal with the conflicting labels?
>>
>> No, just do not match SSA_NAME. And instead of
>>
>> + (with { gimple *def_stmt = SSA_NAME_DEF_STMT (@0); }
>> + (if (is_gimple_assign (def_stmt)
>> + && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
>>
>> you instead want to change the pattern to
>>
>> (simpify
>> (cmp (convert @0) INTEGER_CST@1)
>>
>> @0 will then be your innerop
>>
>> note that you can't use get_value_range but you have to use the
>> get_range_info interface instead. I suppose a helper function
>> somewhere that computes whether an expression fits a type
>> would be helpful (see expr_not_equal_to for sth related,
>> thus expr_fits_type_p (@0, TREE_TYPE (@1)))
>
> All right, I think we got that (new patch attached).
>
>> Likewise the overflow_infinity checks do not translate to match.pd
>> (or rahter the range info you get).
>
> Can you give us another hint here, please? The overflow check
> should probably go into expr_fits_type_p, but with only the min
> and max values from get_range_info, how can the overflow
> TREE_OVERFLOW_P flag be retrieved from @1, to duplicate the logic
> from is_{nega,posi}tive_overflow_infinity? Is it availably
> somewhere, or is it necessary to somehow re-calculate it from the
> expression?
You can't - just drop it (I've been dropping those already).
> (This is really necessary so that cases like this don't start
> folding with the patch:
>
> --
> signed char foo3uu (unsigned char a)
> {
> unsigned char d;
> unsigned long un;
>
> d = (a & 63) + 200;
> un = d;
> if (un >= 12)
> ubar(un);
>
> return d;
> }
as Marc said there is nothing wrong with this folding.
+ if (has_range)
+ {
+ if (sign == tsign)
+ {
+ if (wi::le_p (max, TYPE_MAX_VALUE (type), sign)
+ && wi::ge_p (min, TYPE_MIN_VALUE (type), sign))
+ return true;
...
I believe you need to use widest_ints here, otherwise mixed precision
max / type will either lead to ICEs or truncations. You then can also
always resort to signed compares.
from your other mail:
(for cmp (simple_comparison)
(simplify
(cmp (convert@0 SSA_NAME@1) INTEGER_CST@2)
(if (!POINTER_TYPE_P (TREE_TYPE (@1))
&& !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (@1)
&& desired_pro_or_demotion_p (TREE_TYPE (@1), TREE_TYPE (@0)))
&& expr_fits_type_p (@1, TREE_TYPE (@0))
&& int_fits_type_p (@2, TREE_TYPE (@1))
&& (!has_single_use (@1) || has_single_use (@0)))
(cmp @1 (convert @2))))))
that looks good now.
Thanks,
Richard.
> --
>
> )
>
> Ciao
>
> Dominik ^_^ ^_^
>
> --
>
> Dominik Vogt
> IBM Germany
