Re: [PATCH] Fix c/69643, named address space wrong-code

[Richard Biener <richard dot guenther at gmail dot com>]

On February 4, 2016 10:04:47 PM GMT+01:00, Richard Henderson <rth@redhat.com> wrote:
>On 02/04/2016 10:07 PM, Richard Biener wrote:
>> On Wed, Feb 3, 2016 at 10:44 PM, Richard Henderson <rth@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)));
>
>This version fails to fall through to the next code block when
>   (1) Both types are pointers,
>   (2) Both types have the same address space,
>which will do the wrong thing when
>   (3) The pointers have different modes.
>
>Recall that several ports allow multiple modes for pointers.

Oh, I thought they would be different address spaces.  So we'd need to add a mode check as well.  I hope we don't have different type bit-precision with the same mode for pointers here?

Richard.

>> with the goal to get rid of special-casing ADDR_SPACE_GENERIC in GCC
>7
>> (and thus analyzing/fixing the folding regression).
>
>Sure.
>
>
>r~