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))))
Richard.
> PS: Writing changelog entries for match.pd is a bit tedious.
> Should we add optional names for pattern so we can refer to them by names
> in the ChangeLog for the more complicated ones ?
> Or maybe just use comments:
> (simplify /* name */ ... ) -;)
That will add the fun of inventing names ;)
> Thanks,
> Prathamesh
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
