https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63580

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org

--- Comment #5 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Hmm, I think we may get confused by having ADDRESSABLE flag set because the
original function body took its address and it makes sense to clear it as
Martin suggest.
I am bit confused on why this leads to ICE - I think false positive on
ADDRESSABLE flag ought to make cgraphunit to introduce extra copy in:

      if (nargs)
        for (i = 1, arg = DECL_CHAIN (a); i < nargs; i++, arg = DECL_CHAIN
(arg))
          {
            tree tmp = arg;
            if (!is_gimple_val (arg))
              {
                tmp = create_tmp_reg (TYPE_MAIN_VARIANT
                                      (TREE_TYPE (arg)), "arg");
                gimple stmt = gimple_build_assign (tmp, arg);
                gsi_insert_after (&bsi, stmt, GSI_NEW_STMT);
              }
            vargs.quick_push (tmp);
          }

that may later end up on stack and prevent call from being a tail call.

I see that later optimizers may not be able to optimize out the extra copy
simply because it is bit hard to do: one needs to verify that the original
location is dead and possibly may be rewritten by a callee.

I did not find any code that would prevent clearning ADDRESSABLE flag on
parameters, so I think Martin's patch is correct.

Additionally I think it is not safe to set TAILCALL flag if the code above
introduced a local variable. This is becuase setting this flag by hand bypasses
logic in suitable_for_tail_call_opt_p that prevents tail call conversion when
any ADDRESSABLE vars exists.

I did not find any reasons why ADDRESSABLE flag should be set on parameter
besides the usual case that it has address taken, so I tink Martin's patch is
OK and correct fix.

Martin, does it fix the other PR too?

Reply via email to