On Tue, May 21, 2013 at 4:50 PM, Richard Biener <richard.guent...@gmail.com> wrote: > On Tue, May 21, 2013 at 8:38 AM, Bin.Cheng <amker.ch...@gmail.com> wrote: >> On Tue, May 21, 2013 at 1:55 PM, Andrew Pinski <pins...@gmail.com> wrote: >>> On Mon, May 20, 2013 at 10:50 PM, Bin.Cheng <amker.ch...@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.
