Fix gimple_expr_code?

[Richard Biener]

On November 12, 2020 9:43:52 PM GMT+01:00, Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>So I spent some time tracking down a ranger issue, and in the end, it 
>boiled down to the range-op handler not being picked up properly.
>
>The handler is picked up by:
>
>   if ((gimple_code (s) == GIMPLE_ASSIGN) || (gimple_code (s) == 
>GIMPLE_COND))
>    return range_op_handler (gimple_expr_code (s), gimple_expr_type
>(s));

IMHO this should use more specific functions. Gimple_expr_code should go away similar to gimple_expr_type. 

>where it is indexing the table with the gimple_expr_code..
>the stmt being processed was for a pointer assignment,
>   _5 = _33
>and it was coming back with a gimple_expr_code of  VAR_DECL instead of 
>an SSA_NAME... which confused me greatly.
>
>
>gimple_expr_code (const gimple *stmt)
>{
>   enum gimple_code code = gimple_code (stmt);
>   if (code == GIMPLE_ASSIGN || code == GIMPLE_COND)
>     return (enum tree_code) stmt->subcode;
>
>A little more digging shows this:
>
>static inline enum tree_code
>gimple_assign_rhs_code (const gassign *gs)
>{
>   enum tree_code code = (enum tree_code) gs->subcode;
>   /* While we initially set subcode to the TREE_CODE of the rhs for
>      GIMPLE_SINGLE_RHS assigns we do not update that subcode to stay
>      in sync when we rewrite stmts into SSA form or do SSA 
>propagations.  */
>   if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS)
>     code = TREE_CODE (gs->op[1]);
>
>   return code;
>}
>
>Fascinating comment.

... 😬 

>But it means that gimple_expr_code() isn't returning the correct result
>
>for GIMPLE_SINGLE_RHS....

It depends. A SSA name isn't an expression code either. As said, the generic gimple_expr_code should be used with extreme care. 

>Wouldn't it make sense that gimple_expr_code be changed to return 
>gimple_assign_rhs_code() for GIMPLE_ASSIGN?
>
>I tested the attached patch, and it bootstraps and passes regression
>tests.
>
>There aren't a lot of places where its used, but I saw a suspicious bit
>
>in ipa-icf-gimple.c that looks like it is working around this?
>
>
>    bool
>    func_checker::compare_gimple_assign (gimple *s1, gimple *s2)
>    {
>       tree arg1, arg2;
>       tree_code code1, code2;
>       unsigned i;
>
>       code1 = gimple_expr_code (s1);
>       code2 = gimple_expr_code (s2);
>
>       if (code1 != code2)
>         return false;
>
>       code1 = gimple_assign_rhs_code (s1);
>       code2 = gimple_assign_rhs_code (s2);
>
>       if (code1 != code2)
>         return false;
>
>
>and  there were one or two other places where SSA_NAME occurred in the 
>cases of a switch after calling gimple_expr_code().
>
>This seems like it should be the right thing?
>Andrew