[PATCH] Fix (intptr_t) x eq/ne CST to x eq/ne (typeof x) cst match.pd pattern (PR tree-optimization/85446)

[Marc Glisse]

On Thu, 19 Apr 2018, Richard Biener wrote:

> On Thu, 19 Apr 2018, Marc Glisse wrote:
>
>> On Thu, 19 Apr 2018, Jakub Jelinek wrote:
>>
>>> As mentioned in the PR, this optimization can't work if @0's precision
>>> is higher than @1's precision, because originally it compares just some set
>>> of lower bits, but in the new comparison compares all bits.
>>> If @0's precision is smaller than @1's precision (in this case @0 can't be
>>> a pointer, as we disallow such direct casts), then in theory it can be
>>> handled, but will not match what the comment says and we'd need to verify
>>> that the @1 constant can be represented in the @0's precision.
>>>
>>> This patch just verifies the precision is the same and does small formatting
>>> cleanup.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>>> trunk?
>>
>> That certainly seems safe, but I am surprised to see a direct cast from 64-bit
>> pointer to 32-bit integer. I've always seen gcc represent those with an
>> intermediate cast to a 64-bit integer, even if verify_gimple_assign_unary
>> allows the direct cast. Does it depend on the platform? It might be nice to
>> canonicalize this a bit, either by forbidding narrowing pointer-to-integer
>> casts, or by simplifying cast chains to direct casts.
>
> We are only (well, that was the intention until I broke the verifier...)
> disallowing widening casts from pointers because whether there is
> zero- or sign-extension involved isn't specified (in fact TYPE_SIGN
> of the pointer isn't what matters here but POINTERS_EXTEND_UNSIGNED,
> and that's even not well-defined for random address-spaces I think).
>
> Not sure if it's really required to restrict things further.

Then we should probably go with option 2 "simplifying cast chains to 
direct casts". Currently,

   unsigned f(char*p){return p;}

is turned into

   p.0_1 = (long int) p_2(D);
   _3 = (unsigned int) p.0_1;

instead of the simpler (more canonical?)

   _3 = (unsigned int) p_2(D);

(ideally to me, the type should be part of the operations more than the 
objects, so "p.0_1 = (long int) p_2(D)" would just be a copy and not a 
(nop) conversion, but that would be way too big a change)

-- 
Marc Glisse