Question on operand_equal_p on different type conversion expressions

[Bin.Cheng]

On Tue, May 21, 2013 at 4:50 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, May 21, 2013 at 8:38 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Tue, May 21, 2013 at 1:55 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Mon, May 20, 2013 at 10:50 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>
>>>
>>>
>>> NOP_EXPR here is a misnamed tree really.  It could also be a
>>> CONVERT_EXPR and still have the same issue as the types are not the
>>> same.
>>>
>>>
>>>> The problem is operand_equal_q simply return false because arg0/arg1
>>>> have different tree code.
>>>
>>> No it returns false because the types are two different.  One is
>>> signed and the other is unsigned.
>>>
>>>>
>>>> Should operand_equal_q take two kinds of conversion expression into
>>>> consideration, or arg0/arg1 are not equal? Thanks.
>>>
>>> Yes why would it not?  Look at the resulting types again.
>>
>> Thanks very much. The dumped tree codes are different (my mistake).
>> But the problem still exists in operand_equal_q.
>> For now with below tree nodes,
>> arg0:
>> <convert_expr 0xb72ddb04
>>     type <integer_type 0xb74602a0 short int sizes-gimplified public HI
>>         size <integer_cst 0xb744e7c4 constant 16>
>>         unit size <integer_cst 0xb744e7e0 constant 2>
>>         align 16 symtab 0 alias set 4 canonical type 0xb74602a0
>> precision 16 min <integer_cst 0xb744e770 -32768> max <integer_cst
>> 0xb744e78c 32767> context <translation_unit_decl 0xb760dd80 D.6120>
>>         pointer_to_this <pointer_type 0xb7241600>>
>>
>>     arg 0 <ssa_name 0xb72882f8
>>         type <integer_type 0xb7460420 long int sizes-gimplified public SI
>>             size <integer_cst 0xb744e55c constant 32>
>>             unit size <integer_cst 0xb744e578 constant 4>
>>             align 32 symtab 0 alias set 5 canonical type 0xb7460420
>> precision 32 min <integer_cst 0xb744e888 -2147483648> max <integer_cst
>> 0xb744e8a4 2147483647> context <translation_unit_decl 0xb760dd80
>> D.6120>
>>             pointer_to_this <pointer_type 0xb74677e0>>
>>         visiteddef_stmt _23 = *_22;
>>
>>         version 23>>
>>
>> arg1:
>> <nop_expr 0xb72e1b54
>>     type <integer_type 0xb74602a0 short int sizes-gimplified public HI
>>         size <integer_cst 0xb744e7c4 constant 16>
>>         unit size <integer_cst 0xb744e7e0 constant 2>
>>         align 16 symtab 0 alias set 4 canonical type 0xb74602a0
>> precision 16 min <integer_cst 0xb744e770 -32768> max <integer_cst
>> 0xb744e78c 32767> context <translation_unit_decl 0xb760dd80 D.6120>
>>         pointer_to_this <pointer_type 0xb7241600>>
>>
>>     arg 0 <ssa_name 0xb72882f8
>>         type <integer_type 0xb7460420 long int sizes-gimplified public SI
>>             size <integer_cst 0xb744e55c constant 32>
>>             unit size <integer_cst 0xb744e578 constant 4>
>>             align 32 symtab 0 alias set 5 canonical type 0xb7460420
>> precision 32 min <integer_cst 0xb744e888 -2147483648> max <integer_cst
>> 0xb744e8a4 2147483647> context <translation_unit_decl 0xb760dd80
>> D.6120>
>>             pointer_to_this <pointer_type 0xb74677e0>>
>>         visiteddef_stmt _23 = *_22;
>>
>>         version 23>>
>>
>> The operand_equal_p still returns false because below piece of code in it:
>>
>> #if 1
>>   if (TREE_CODE (arg0) != TREE_CODE (arg1)
>>       /* This is needed for conversions and for COMPONENT_REF.
>>      Might as well play it safe and always test this.  */
>>       || TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
>>       || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
>>       || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))
>> #else
>>   if ((TREE_CODE (arg0) != TREE_CODE (arg1)
>>        && !(CONVERT_EXPR_P (arg0) && CONVERT_EXPR_P (arg1)))
>>       /* This is needed for conversions and for COMPONENT_REF.
>>      Might as well play it safe and always test this.  */
>>       || TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
>>       || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
>>       || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))
>> #endif
>>     return 0;
>>
>>
>> The code in else part should be used instead, right?
>
> As this code is used by frontends who may actually have different behaviors
> for NOP_EXPR vs. CONVERT_EXPR (which is the only reason the two
> tree codes still exist!) it isn't 100% obvious.  Though if it passes testing
> then it's ok IMHO, but I'd like you to refactor the above to
>
>     if (TREE_CODE (arg0) != TREE_CODE (arg1)
>         /* NOP_EXPR and CONVERT_EXPR are considered equal.  */
>         && !(CONVERT_EXPR_P (arg0) && CONVERT_EXPR_P (arg1)))
>       return 0;
>
>     /* This is needed for conversions and for COMPONENT_REF.
>          Might as well play it safe and always test this.  */
>     if (TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
>       || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
>       || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))
>       return 0;
>
> ok if that passes testing.
>
Will be done. Thanks.


--
Best Regards.