Allow entry point markers without debug support in accelerator compiler

Hello

A number of the libgomp tests running with AMD GCN offloading fail with 
the following internal compiler error:

during RTL pass: final
/scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/libgomp/testsuite/libgomp.fortran/examples-4/async_target-1.f90: 
In function 'pipedf_._omp_fn.2':
/scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/libgomp/testsuite/libgomp.fortran/examples-4/async_target-1.f90:49: 
internal compiler error: in dwarf2out_inline_entry, at dwarf2out.c:27682
0x626210 dwarf2out_inline_entry
	/scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/dwarf2out.c:27682
0x9692c4 final_scan_insn_1
	/scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:2435
0x969f4b final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
	/scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:3152
0x96a214 final_1
	/scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:2020
0x96ac7f rest_of_handle_final
	/scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:4658
0x96ac7f execute
	/scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:4736

The ICE is due to an assert for debug_inline_points firing. The test 
case does not explicitly set this (using -ginline-points), so it is 
auto-detected.

The problem arises because the host compiler enables it by default, but 
the offload compiler disables it. The host compiler generates the Gimple 
debug statements for inlined functions, then streams them out using the 
LTO mechanism. The accelerator compiler streams them in, encounters the 
unexpected debug statements and ICEs due to a failed assertion.

It is possible to make GCN enable support inline-points by default, but 
I think it would be better to fix it for the general case where there is 
a disagreement between host and accelerator? This patch makes 
dwarf2out_inline_entry ignore the inline entry note if 
debug_inline_points is not set while the compiler is in LTO mode. This 
is effectively relaxing the assertion condition by allowing an exception 
for LTO.

Bootstrapped on x86_64, and tested using GCN as an offload accelerator. 
Okay for trunk?

Kwok


It is possible for the host compiler to emit entry point markers in the
GIMPLE code while the accelerator compiler has them disabled, causing an
assertion to fire where processed by the accelerator compiler.  This is
fixed by allowing the markers to be ignored in LTO mode only.

2019-12-06  Kwok Cheung Yeung  <kcy@codesourcery.com>

	gcc/
	* dwarf2out.c (dwarf2out_inline_entry): Return early if in LTO and
	debug_inline_points not set.
---
  gcc/dwarf2out.c | 7 +++++++
  1 file changed, 7 insertions(+)

    gcc_assert (debug_inline_points);

    /* If we can't represent it, don't bother.  */

On Fri, Dec 6, 2019 at 6:27 PM Kwok Cheung Yeung <kcy@codesourcery.com> wrote:
>
> Hello
>
> A number of the libgomp tests running with AMD GCN offloading fail with
> the following internal compiler error:
>
> during RTL pass: final
> /scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/libgomp/testsuite/libgomp.fortran/examples-4/async_target-1.f90:
> In function 'pipedf_._omp_fn.2':
> /scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/libgomp/testsuite/libgomp.fortran/examples-4/async_target-1.f90:49:
> internal compiler error: in dwarf2out_inline_entry, at dwarf2out.c:27682
> 0x626210 dwarf2out_inline_entry
>         /scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/dwarf2out.c:27682
> 0x9692c4 final_scan_insn_1
>         /scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:2435
> 0x969f4b final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
>         /scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:3152
> 0x96a214 final_1
>         /scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:2020
> 0x96ac7f rest_of_handle_final
>         /scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:4658
> 0x96ac7f execute
>         /scratch/ci-cs/amdtest/upstream-offload/src/gcc-mainline/gcc/final.c:4736
>
> The ICE is due to an assert for debug_inline_points firing. The test
> case does not explicitly set this (using -ginline-points), so it is
> auto-detected.
>
> The problem arises because the host compiler enables it by default, but
> the offload compiler disables it. The host compiler generates the Gimple
> debug statements for inlined functions, then streams them out using the
> LTO mechanism. The accelerator compiler streams them in, encounters the
> unexpected debug statements and ICEs due to a failed assertion.
>
> It is possible to make GCN enable support inline-points by default, but
> I think it would be better to fix it for the general case where there is
> a disagreement between host and accelerator? This patch makes
> dwarf2out_inline_entry ignore the inline entry note if
> debug_inline_points is not set while the compiler is in LTO mode. This
> is effectively relaxing the assertion condition by allowing an exception
> for LTO.
>
> Bootstrapped on x86_64, and tested using GCN as an offload accelerator.
> Okay for trunk?

The stream-in code has

          /* If we're recompiling LTO objects with debug stmts but
             we're not supposed to have debug stmts, remove them now.
             We can't remove them earlier because this would cause uid
             mismatches in fixups, but we can do it at this point, as
             long as debug stmts don't require fixups.
             Similarly remove all IFN_*SAN_* internal calls   */
          if (!flag_wpa)
            {
              if (is_gimple_debug (stmt)
                  && (gimple_debug_nonbind_marker_p (stmt)
                      ? !MAY_HAVE_DEBUG_MARKER_STMTS
                      : !MAY_HAVE_DEBUG_BIND_STMTS))
                remove = true;
              /* In case the linemap overflows locations can be dropped
                 to zero.  Thus do not keep nonsensical inline entry markers
                 we'd later ICE on.  */
              tree block;
              if (gimple_debug_inline_entry_p (stmt)
                  && (block = gimple_block (stmt))
                  && !inlined_function_outer_scope_p (block))
                remove = true;

so can you please instead amend that or figure why it doesn't work?

Thanks,
Richard.

> Kwok
>
>
> It is possible for the host compiler to emit entry point markers in the
> GIMPLE code while the accelerator compiler has them disabled, causing an
> assertion to fire where processed by the accelerator compiler.  This is
> fixed by allowing the markers to be ignored in LTO mode only.
>
> 2019-12-06  Kwok Cheung Yeung  <kcy@codesourcery.com>
>
>         gcc/
>         * dwarf2out.c (dwarf2out_inline_entry): Return early if in LTO and
>         debug_inline_points not set.
> ---
>   gcc/dwarf2out.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 6fb345b..44fa071 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -27679,6 +27679,13 @@ block_within_block_p (tree block, tree outer,
> bool bothways)
>   static void
>   dwarf2out_inline_entry (tree block)
>   {
> +  /* In an offloading configuration, it is possible for the host
> toolchain but
> +     not the offload toolchain to support extended debug information
> for inlined
> +     functions.  In that case, we can just ignore any entry point markers
> +     read from the LTO stream.  */
> +  if (in_lto_p && !debug_inline_points)
> +    return;
> +
>     gcc_assert (debug_inline_points);
>
>     /* If we can't represent it, don't bother.  */
> --
> 2.8.1
>

Hello

On 09/12/2019 8:01 am, Richard Biener wrote:
> 
> The stream-in code has
> 
>            /* If we're recompiling LTO objects with debug stmts but
>               we're not supposed to have debug stmts, remove them now.
>               We can't remove them earlier because this would cause uid
>               mismatches in fixups, but we can do it at this point, as
>               long as debug stmts don't require fixups.
>               Similarly remove all IFN_*SAN_* internal calls   */
>            if (!flag_wpa)
>              {
>                if (is_gimple_debug (stmt)
>                    && (gimple_debug_nonbind_marker_p (stmt)
>                        ? !MAY_HAVE_DEBUG_MARKER_STMTS
>                        : !MAY_HAVE_DEBUG_BIND_STMTS))
>                  remove = true;
>                /* In case the linemap overflows locations can be dropped
>                   to zero.  Thus do not keep nonsensical inline entry markers
>                   we'd later ICE on.  */
>                tree block;
>                if (gimple_debug_inline_entry_p (stmt)
>                    && (block = gimple_block (stmt))
>                    && !inlined_function_outer_scope_p (block))
>                  remove = true;
> 
> so can you please instead amend that or figure why it doesn't work?

Thank you for pointing out that code. I have extended it to also strip 
out the inline entry markers if debug_inline_points is false in the 
accelerator compiler, which prevents execution from ever reaching the 
assert.

I have rerun the libgomp tests on AMD GCN with no regressions. Okay for 
trunk?

Thanks

Kwok


2020-01-09  Kwok Cheung Yeung  <kcy@codesourcery.com>

	gcc/
	* lto-streamer-in.c (input_function): Remove streamed-in inline debug
	markers	if debug_inline_points is false.
---
  gcc/lto-streamer-in.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
index a8d67c4..3e64371 100644
--- a/gcc/lto-streamer-in.c
+++ b/gcc/lto-streamer-in.c
@@ -1135,8 +1135,9 @@ input_function (tree fn_decl, class data_in *data_in,
  		 we'd later ICE on.  */
  	      tree block;
  	      if (gimple_debug_inline_entry_p (stmt)
-		  && (block = gimple_block (stmt))
-		  && !inlined_function_outer_scope_p (block))
+		  && (((block = gimple_block (stmt))
+		       && !inlined_function_outer_scope_p (block))
+		      || !debug_inline_points))
  		remove = true;
  	      if (is_gimple_call (stmt)
  		  && gimple_call_internal_p (stmt))

On Thu, Jan 9, 2020 at 2:35 PM Kwok Cheung Yeung <kcy@codesourcery.com> wrote:
>
> Hello
>
> On 09/12/2019 8:01 am, Richard Biener wrote:
> >
> > The stream-in code has
> >
> >            /* If we're recompiling LTO objects with debug stmts but
> >               we're not supposed to have debug stmts, remove them now.
> >               We can't remove them earlier because this would cause uid
> >               mismatches in fixups, but we can do it at this point, as
> >               long as debug stmts don't require fixups.
> >               Similarly remove all IFN_*SAN_* internal calls   */
> >            if (!flag_wpa)
> >              {
> >                if (is_gimple_debug (stmt)
> >                    && (gimple_debug_nonbind_marker_p (stmt)
> >                        ? !MAY_HAVE_DEBUG_MARKER_STMTS
> >                        : !MAY_HAVE_DEBUG_BIND_STMTS))
> >                  remove = true;
> >                /* In case the linemap overflows locations can be dropped
> >                   to zero.  Thus do not keep nonsensical inline entry markers
> >                   we'd later ICE on.  */
> >                tree block;
> >                if (gimple_debug_inline_entry_p (stmt)
> >                    && (block = gimple_block (stmt))
> >                    && !inlined_function_outer_scope_p (block))
> >                  remove = true;
> >
> > so can you please instead amend that or figure why it doesn't work?
>
> Thank you for pointing out that code. I have extended it to also strip
> out the inline entry markers if debug_inline_points is false in the
> accelerator compiler, which prevents execution from ever reaching the
> assert.
>
> I have rerun the libgomp tests on AMD GCN with no regressions. Okay for
> trunk?

OK.

Thanks,
Richard.

> Thanks
>
> Kwok
>
>
> 2020-01-09  Kwok Cheung Yeung  <kcy@codesourcery.com>
>
>         gcc/
>         * lto-streamer-in.c (input_function): Remove streamed-in inline debug
>         markers if debug_inline_points is false.
> ---
>   gcc/lto-streamer-in.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
> index a8d67c4..3e64371 100644
> --- a/gcc/lto-streamer-in.c
> +++ b/gcc/lto-streamer-in.c
> @@ -1135,8 +1135,9 @@ input_function (tree fn_decl, class data_in *data_in,
>                  we'd later ICE on.  */
>               tree block;
>               if (gimple_debug_inline_entry_p (stmt)
> -                 && (block = gimple_block (stmt))
> -                 && !inlined_function_outer_scope_p (block))
> +                 && (((block = gimple_block (stmt))
> +                      && !inlined_function_outer_scope_p (block))
> +                     || !debug_inline_points))
>                 remove = true;
>               if (is_gimple_call (stmt)
>                   && gimple_call_internal_p (stmt))
> --
> 2.8.1
>