Re: Go PATCH to fix libgo breakage (PR tree-optimization/67284)

[Ian Lance Taylor <iant at golang dot org>]

On Mon, Aug 24, 2015 at 4:34 AM, Marek Polacek <polacek@redhat.com> wrote:
>
> This is hopefully the last attempt to fix the libgo breakage.  This time
> around I really think I got to the bottom of the problem.  The issue is
> that the Go FE defines the __builtin_trap call, but doesn't set the
> TREE_THIS_VOLATILE flag on it.  That's bad because then this call isn't
> gimple_call_noreturn_p and the cgraph cleanup code can't do its job properly.
>
> Bootstrapped/regtested on x86_64-linux.  Now, I don't know the Go patch
> process here.  Should I just wait for you Ian to commit the patch?
> (Yeah, I think I don't need the ChangeLog entry.)

For files in gcc/go/gofrontend, you should wait for me to commit the
path.  This file is in gcc/go, not gcc/go/gofrontend, so you can
commit the patch directly, with a ChangeLog entry.  (I know it's
confusing.)


> 2015-08-24  Marek Polacek  <polacek@redhat.com>
>
>         PR tree-optimization/67284
>         * go-gcc.cc (Gcc_backend::define_builtin): Add NORETURN_P default
>         argument.  Set TREE_THIS_VOLATILE.
>         (Gcc_backend::Gcc_backend): Mark __builtin_trap as a noreturn call.
>
> diff --git gcc/go/go-gcc.cc gcc/go/go-gcc.cc
> index 6f274fc..aabcd98 100644
> --- gcc/go/go-gcc.cc
> +++ gcc/go/go-gcc.cc
> @@ -498,7 +498,7 @@ class Gcc_backend : public Backend
>  private:
>    void
>    define_builtin(built_in_function bcode, const char* name, const char* libname,
> -                tree fntype, bool const_p);
> +                tree fntype, bool const_p, bool noreturn_p = false);

I would rather not see a default value for this parameter.  I would
rather see all the calls change to pass false.  As the GCC coding
conventions say, one should normally avoid default arguments
(https://gcc.gnu.org/codingconventions.html#Default).

This patch is OK with that change.

Thanks.

Ian