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]

Re: [PR70920] transform (intptr_t) x eq/ne CST to x eq/ne (typeof x) cst


On 26 July 2016 at 17:28, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
>
>> On 25 July 2016 at 14:32, Richard Biener <rguenther@suse.de> wrote:
>> > On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
>> >
>> >> Hi Richard,
>> >> The attached patch tries to fix PR70920.
>> >> It adds your pattern from comment 1 in the PR
>> >> (with additional gating on INTEGRAL_TYPE_P to avoid regressing finalize_18.f90)
>> >> and second pattern, which is reverse of the first transform.
>> >> I needed to update ssa-dom-branch-1.c because with patch applied,
>> >> jump threading removed the second if (i != 0B) block.
>> >> The dumps with and without patch for ssa-dom-branch-1.c start
>> >> to differ with forwprop1:
>> >>
>> >> before:
>> >>  <bb 3>:
>> >>   _1 = temp_16(D)->code;
>> >>   _2 = _1 == 42;
>> >>   _3 = (int) _2;
>> >>   _4 = (long int) _3;
>> >>   temp_17 = (struct rtx_def *) _4;
>> >>   if (temp_17 != 0B)
>> >>     goto <bb 4>;
>> >>   else
>> >>     goto <bb 8>;
>> >>
>> >> after:
>> >> <bb 3>:
>> >>   _1 = temp_16(D)->code;
>> >>   _2 = _1 == 42;
>> >>   _3 = (int) _2;
>> >>   _4 = (long int) _2;
>> >>   temp_17 = (struct rtx_def *) _4;
>> >>   if (_1 == 42)
>> >>     goto <bb 4>;
>> >>   else
>> >>     goto <bb 8>;
>> >>
>> >> I suppose the transform is correct for above test-case ?
>> >>
>> >> Then vrp dump shows:
>> >>  Threaded jump 5 --> 9 to 13
>> >>   Threaded jump 8 --> 9 to 13
>> >>   Threaded jump 3 --> 9 to 13
>> >>   Threaded jump 12 --> 9 to 14
>> >> Removing basic block 9
>> >> basic block 9, loop depth 0
>> >>  pred:
>> >> if (i1_10(D) != 0B)
>> >>   goto <bb 10>;
>> >> else
>> >>   goto <bb 11>;
>> >>  succ:       10
>> >>              11
>> >>
>> >> So there remained two instances of if (i1_10 (D) != 0B) in dom2 dump file,
>> >> and hence needed to update the test-case.
>> >>
>> >> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >> OK to commit ?
>> >
>> > --- a/gcc/match.pd
>> > +++ b/gcc/match.pd
>> > @@ -3408,3 +3408,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>> >          { CONSTRUCTOR_ELT (ctor, idx / k)->value; })
>> >         (BIT_FIELD_REF { CONSTRUCTOR_ELT (ctor, idx / k)->value; }
>> >                        @1 { bitsize_int ((idx % k) * width); })))))))))
>> > +
>> > +/* PR70920: Transform (intptr_t)x eq/ne CST to x eq/ne (typeof x) CST.
>> > */
>> > +
>> > +(for cmp (ne eq)
>> > + (simplify
>> > +  (cmp (convert@2 @0) INTEGER_CST@1)
>> > +  (if (POINTER_TYPE_P (TREE_TYPE (@0))
>> > +       && INTEGRAL_TYPE_P (TREE_TYPE (@2)))
>> >
>> > you can use @1 here and omit @2.
>> >
>> > +   (cmp @0 (convert @1)))))
>> > +
>> > +/* Reverse of the above case:
>> > +   x has integral_type, CST is a pointer constant.
>> > +   Transform (typeof CST)x eq/ne CST to x eq/ne (typeof x) CST.  */
>> > +
>> > +(for cmp (ne eq)
>> > + (simplify
>> > +  (cmp (convert @0) @1)
>> > +  (if (POINTER_TYPE_P (TREE_TYPE (@1))
>> > +       && INTEGRAL_TYPE_P (TREE_TYPE (@0)))
>> > +    (cmp @0 (convert @1)))))
>> >
>> > The second pattern lacks the INTEGER_CST on @1 so it doesn't match
>> > its comment.  Please do not add vertical space between pattern
>> > comment and pattern.
>> >
>> > Please place patterns not at the end of match.pd but where similar
>> > transforms are done.  Like after
>> >
>> > /* Simplify pointer equality compares using PTA.  */
>> > (for neeq (ne eq)
>> >  (simplify
>> >   (neeq @0 @1)
>> >   (if (POINTER_TYPE_P (TREE_TYPE (@0))
>> >        && ptrs_compare_unequal (@0, @1))
>> >    { neeq == EQ_EXPR ? boolean_false_node : boolean_true_node; })))
>> >
>> > please also share the (for ...) for both patterns or merge them
>> > by changing the condition to
>> >
>> >   (if ((POINTER_TYPE_P (TREE_TYPE (@0))
>> >         && INTEGRAL_TYPE_P (TREE_TYPE (@1)))
>> >        || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> >            && POINTER_TYPE_P (TREE_TYPE (@1))))
>> >
>> Hi,
>> Done suggested changes in this version.
>> pr70920-4.c (test-case in patch) is now folded during  ccp instead of
>> forwprop after merging the
>> two patterns.
>> Passes bootstrap+test on x86_64-unknown-linux-gnu.
>> OK for trunk ?
>
> (please paste in ChangeLog entries rather than attaching them).
Will do henceforth.
>
> In gcc.dg/tree-ssa/ssa-dom-branch-1.c you need to adjust the comment
> before the dump-scan you adjust.
>
> Ok with that change.
Thanks, committed as r238754 after adjusting the comment in ssa-dom-branch-1.c.

Thanks,
Prathamesh
>
> Thanks,
> Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]