Re: [PATCH] Fix PR ipa/65318

[Richard Biener <richard.guenther@gmail.com>]

On Thu, Mar 5, 2015 at 7:38 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >Index: gimple-fold.c
>> >===================================================================
>> >--- gimple-fold.c    (revision 221170)
>> >+++ gimple-fold.c    (working copy)
>> >@@ -263,7 +263,16 @@ get_symbol_constant_value (tree sym)
>> >     {
>> >       val = canonicalize_constructor_val (unshare_expr (val), sym);
>> >       if (val && is_gimple_min_invariant (val))
>> >-        return val;
>> >+        {
>> >+              if (!useless_type_conversion_p (TREE_TYPE (sym),
>> >TREE_TYPE (val)))
>> >+            {
>> >+              if (operand_equal_p (TYPE_SIZE (TREE_TYPE (sym)),
>> >+                                   TYPE_SIZE (TREE_TYPE (val)), 0))
>> >+                return NULL_TREE;
>>
>> And no, I don't think this is sane.  Callers need to handle mismatches IIRC.
>
> OK, I am little bit confused about your MEM_REF suggestion. So you mean that
> MEM_REF should be added around all references to symbol that is an alias?
> Where it is done?  Is there a reason why we do not add MEM_REF always?  I would
> like to keep optimization passes (like ipa-visibility or ICF) to turn symbol
> into an alias without having to update underlying IL.

Yes - but I said that having an alias should have the same effect as
the MEM_REF wrapping we do in LTO (to not barf on stmt verification
if symbol merging merges an int and a float for example).

>
> Concerning callers handling mismatches, the VIEW_CONVERT_EXPR thing seems valid
> thing to do for all uses except for fold_const_aggregate_ref_1. So perhaps
> we can just inline rest of get_symbol_constant_value in there and document that
> get_symbol_constant_value returns value in correct type.
>
> Or am I missing something obvious?

Yeah, that looks good.  Note that we can as well change all callers
of get_symbol_constant_value to use fold_const_aggregate_ref, no?
So reduce the number of APIs.

Richard.

>
> Thanks!
> Honza
>>
>> >+              val = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (sym), val);
>> >+            }
>> >+          return val;
>> >+        }
>> >       else
>> >         return NULL_TREE;
>> >     }
>>