On Tue, May 21, 2013 at 8:38 AM, Bin.Cheng <[email protected]> wrote:
> On Tue, May 21, 2013 at 1:55 PM, Andrew Pinski <[email protected]> wrote:
>> On Mon, May 20, 2013 at 10:50 PM, Bin.Cheng <[email protected]> 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.
Thanks,
Richard.
> Thanks.
>
> --
> Best Regards.
