On Wed, Feb 3, 2016 at 10:44 PM, Richard Henderson <r...@redhat.com> wrote:
> On 02/04/2016 07:30 AM, Richard Henderson wrote:
>>
>> On 02/04/2016 12:46 AM, Richard Biener wrote:
>>>
>>> As for a patch I'd repeatedly pondered on not stripping int <-> pointer
>>> conversions at all, similar to what STRIP_SIGN_NOPS does. Don't remember
>>> actually trying this or the fallout though.
>>
>>
>> I'll run that through a test cycle and see what happens.
>
>
>
> +FAIL: c-c++-common/fold-bitand-4.c -Wc++-compat scan-tree-dump-times
> original "& 15" 1
> +FAIL: c-c++-common/fold-bitand-4.c -Wc++-compat scan-tree-dump-times
> original "return [^\\n0-9]*0;" 2
> +FAIL: c-c++-common/fold-bitand-4.c -Wc++-compat scan-tree-dump-times
> original "return [^\\n0-9]*12;" 1
> +FAIL: gcc.dg/fold-bitand-1.c scan-tree-dump-times original "&c4 & 3" 0
> +FAIL: gcc.dg/fold-bitand-1.c scan-tree-dump-times original "&c8 & 3" 0
> +FAIL: gcc.dg/fold-bitand-1.c scan-tree-dump-times original "return 0" 2
> +FAIL: gcc.dg/fold-bitand-2.c scan-tree-dump-times original "& 3" 0
> +FAIL: gcc.dg/fold-bitand-2.c scan-tree-dump-times original "return 0" 1
> +FAIL: gcc.dg/fold-bitand-2.c scan-tree-dump-times original "return 1" 1
> +FAIL: gcc.dg/fold-bitand-2.c scan-tree-dump-times original "return 2" 1
> +FAIL: gcc.dg/fold-bitand-2.c scan-tree-dump-times original "return 3" 1
> +FAIL: gcc.dg/fold-bitand-3.c scan-tree-dump-times original "& 3" 0
> +FAIL: gcc.dg/fold-bitand-3.c scan-tree-dump-times original "return 1" 2
> +FAIL: gcc.dg/pr52355.c (test for excess errors)
> +FAIL: gcc.dg/tree-ssa/foldaddr-1.c scan-tree-dump-times original "return 0"
> 1
> +FAIL: gcc.dg/tree-ssa/ivopt_4.c scan-tree-dump-times ivopts "ivtmp.[0-9_]*
> = PHI <" 1
> +FAIL: gcc.dg/tree-ssa/pr21985.c scan-tree-dump-times optimized "foo
> \\\\([0-9]*\\\\)" 2
> +FAIL: gcc.dg/tree-ssa/pr22051-2.c scan-tree-dump-times optimized "r_. =
> \\\\(int\\\\) q" 1
> +FAIL: gcc.target/i386/addr-space-5.c scan-assembler gs:
>
>
> So, it even fails the new test I added there at the end.
> Patch below, just in case I've misunderstood what you suggested.
Yes, that's what I had suggested. Of course to fix the addr-space issue
it has to add the certainly missing addr-space compatibility fix for
the pointer-pointer cast case. So yes, somewhat what I expected, some
missed fold opportunities which expect the pointer-int cast stripping.
I would guess that in match.pd
/* Two conversions in a row are not needed unless:
- some conversion is floating-point (overstrict for now), or
- some conversion is a vector (overstrict for now), or
- the intermediate type is narrower than both initial and
final, or
- the intermediate type and innermost type differ in signedness,
and the outermost type is wider than the intermediate, or
- the initial type is a pointer type and the precisions of the
intermediate and final types differ, or
- the final type is a pointer type and the precisions of the
initial and intermediate types differ. */
(if (! inside_float && ! inter_float && ! final_float
&& ! inside_vec && ! inter_vec && ! final_vec
&& (inter_prec >= inside_prec || inter_prec >= final_prec)
&& ! (inside_int && inter_int
&& inter_unsignedp != inside_unsignedp
&& inter_prec < final_prec)
&& ((inter_unsignedp && inter_prec > inside_prec)
== (final_unsignedp && final_prec > inter_prec))
&& ! (inside_ptr && inter_prec != final_prec)
&& ! (final_ptr && inside_prec != inter_prec)
&& ! (final_prec != GET_MODE_PRECISION (TYPE_MODE (type))
&& TYPE_MODE (type) == TYPE_MODE (inter_type)))
(ocvt @0))
also needs adjustments. That's to avoid (ptr-w/addr-spaceA
*)(uintptr_t)ptr-w/addr-spaceB
which would strip the integer conversion and thus would require a
ADDR_SPACE_CONVERT_EXPR
(if the addr-spaces are related) or it would be even bogus.
Now looking at your original patch
+ /* Do not strip casts into or out of differing address spaces. */
+ if (POINTER_TYPE_P (outer_type)
+ && TYPE_ADDR_SPACE (TREE_TYPE (outer_type)) != ADDR_SPACE_GENERIC)
+ {
+ if (!POINTER_TYPE_P (inner_type)
+ || (TYPE_ADDR_SPACE (TREE_TYPE (outer_type))
+ != TYPE_ADDR_SPACE (TREE_TYPE (inner_type))))
+ return false;
+ }
+ else if (POINTER_TYPE_P (inner_type)
+ && TYPE_ADDR_SPACE (TREE_TYPE (inner_type)) != ADDR_SPACE_GENERIC)
+ {
+ /* We already know that outer_type is not a pointer with
+ a non-generic address space. */
+ return false;
+ }
it really looks like sth is wrong with our IL if such complications
are necessary here ...
Thus I'd prefer to at least re-write it as
/* Do not strip casts changing the address space to/from
non-ADDR_SPACE_GENERIC. */
if ((POINTER_TYPE_P (outer_type) && TYPE_ADDR_SPACE (TREE_TYPE
(outer_type)) != ADDR_SPACE_GENERIC)
|| (POINTER_TYPE_P (inner_type) && TYPE_ADDR_SPACE (TREE_TYPE
(inner_type)) != ADDR_SPACE_GENERIC))
return (POINTER_TYPE_P (outer_type) && POINTER_TYPE (inner_type)
&& TYPE_ADDR_SPACE (TREE_TYPE (outer_type)) ==
TYPE_ADDR_SPACE (TREE_TYPE (inner_type)));
with the goal to get rid of special-casing ADDR_SPACE_GENERIC in GCC 7
(and thus analyzing/fixing the folding regression).
The above will end up failing to strip the nop conversions in (ptr
*)(uintptr_t)p if both ptr and p have a non-generic address-space.
Of course we now expect folding to deal with conversion sequences and
eventually STRIP_NOPS and friends should be changed
to no longer loop (again, don't remember trying or any actual fallout
in doing that). GCC 7 again...
Thanks,
Richard.
>
>
> r~
>
>
>
> diff --git a/gcc/tree.c b/gcc/tree.c
> index fa7646b..3e79c4b 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -12219,6 +12219,10 @@ block_ultimate_origin (const_tree block)
> bool
> tree_nop_conversion_p (const_tree outer_type, const_tree inner_type)
> {
> + /* Do not strip conversions between pointers and integers. */
> + if (POINTER_TYPE_P (outer_type) != POINTER_TYPE_P (inner_type))
> + return false;
> +
> /* Use precision rather then machine mode when we can, which gives
> the correct answer even for submode (bit-field) types. */
> if ((INTEGRAL_TYPE_P (outer_type)
> @@ -12272,8 +12276,7 @@ tree_sign_nop_conversion (const_tree exp)
> outer_type = TREE_TYPE (exp);
> inner_type = TREE_TYPE (TREE_OPERAND (exp, 0));
>
> - return (TYPE_UNSIGNED (outer_type) == TYPE_UNSIGNED (inner_type)
> - && POINTER_TYPE_P (outer_type) == POINTER_TYPE_P (inner_type));
> + return TYPE_UNSIGNED (outer_type) == TYPE_UNSIGNED (inner_type);
> }
>
> /* Strip conversions from EXP according to tree_nop_conversion and
>
