On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
> On 25 July 2016 at 14:32, Richard Biener <rguent...@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).
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,
Richard.
