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
