Hi!
On 2020-03-23T16:29:40+0100, I wrote:
> On 2018-09-14T10:18:12-0600, Jeff Law <l...@redhat.com> wrote:
>> On 9/5/18 5:52 AM, a...@codesourcery.com wrote:
>>>
>>> The GCN toolchain must use the LLVM assembler and linker because there's no
>>> binutils port. The LLVM tools do not have the same diagnostic style as
>>> binutils
>
> For reference:
> 'libgomp.c++/../libgomp.c-c++-common/function-not-offloaded.c', for
> example:
>
> ld: error: undefined symbol: foo()
> >>> referenced by /tmp/ccNzknBD.o:(main._omp_fn.0)
> >>> referenced by /tmp/ccNzknBD.o:(main._omp_fn.0)
>
> ld: error: undefined symbol: __gxx_personality_v0
> >>> referenced by /tmp/ccNzknBD.o:(.data+0x13)
> collect2: error: ld returned 1 exit status
> mkoffload: fatal error:
> [...]/build-gcc/./gcc/x86_64-pc-linux-gnu-accel-amdgcn-amdhsa-gcc returned 1
> exit status
>
> Note the blank line between the two "diagnostic blocks".
>
>>> so the "blank line(s) in output" tests are inappropriate (and very
>>> noisy).
> So here is a different proposal. The problem we're having is that the
> AMD GCN 'as' (that is, 'llvm-mc') prints "unsuitable" diagnostics (as
> quoted at the top of my email).
(No idea where I got the idea from that 'as' prints 'ld' error messages,
as displayed above...) ;-) (But supposedly that problem applies to both,
or even generally all LLVM tools?)
> How about having a simple wrapper around it, to post-process its 'stderr'
> to remove any blank linkes between "diagnostic blocks"? Then we could
> remove this fragile 'check_effective_target_llvm_binutils' etc.?
>
> I shall offer to implement the simple shell script, and suppose this
> could live in 'gcc/config/gcn/'?
I have implemented that, and it appears to generally work, but of
course...
> ("Just" need to figure out how to
> integrate that into the GCC build process, top-level build system.)
... this was the non-trivial bit, and may need some further thought --
which I can't allocate time for right now, so I'll postpone this for
later. I'm attaching my "[WIP] 'llvm-tools-wrapper' [PR88920]" patch, in
case somebody would like to have a first look.
Grüße
Thomas
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
>From 31c8828d37bed37b4ee4eb3bbefb8eb1db46e0a1 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Tue, 24 Mar 2020 21:59:04 +0100
Subject: [PATCH] [WIP] 'llvm-tools-wrapper' [PR88920]
PR testsuite/88920
---
gcc/Makefile.in | 9 ++
gcc/config.in | 6 +
gcc/configure | 17 +++
gcc/configure.ac | 12 ++
gcc/doc/sourcebuild.texi | 6 -
gcc/gcc.c | 116 +++++++++++++++---
gcc/llvm-tools-wrapper.in | 35 ++++++
gcc/testsuite/lib/gcc-dg.exp | 4 -
gcc/testsuite/lib/target-supports.exp | 15 ---
.../function-not-offloaded.c | 1 -
10 files changed, 175 insertions(+), 46 deletions(-)
create mode 100755 gcc/llvm-tools-wrapper.in
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index fa9923bb2703..4d13707453b2 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1974,6 +1974,7 @@ start.encap: native xgcc$(exeext) cpp$(exeext) specs \
rest.encap: lang.rest.encap
# This is what is made with the host's compiler
# whether making a cross compiler or not.
+#TODO Do we need 'llvm-tools-wrapper' here?
native: config.status auto-host.h build-@POSUB@ $(LANGUAGES) \
$(EXTRA_PROGRAMS) $(COLLECT2) lto-wrapper$(exeext) \
gcc-ar$(exeext) gcc-nm$(exeext) gcc-ranlib$(exeext)
@@ -3525,6 +3526,10 @@ ifeq ($(enable_plugin),yes)
install: install-plugin
endif
+#TODO ifeq (TODO)
+install: install-llvm-tools-wrapper
+#TODO endif
+
install-strip: override INSTALL_PROGRAM = $(INSTALL_STRIP_PROGRAM)
ifneq ($(STRIP),)
install-strip: STRIPPROG = $(STRIP)
@@ -3920,6 +3925,10 @@ install-gcc-ar: installdirs gcc-ar$(exeext) gcc-nm$(exeext) gcc-ranlib$(exeext)
done; \
fi
+# Install llvm-tools-wrapper.
+install-llvm-tools-wrapper: llvm-tools-wrapper$(exeext)
+ $(INSTALL_PROGRAM) llvm-tools-wrapper$(exeext) $(DESTDIR)$(libexecsubdir)/llvm-tools-wrapper$(exeext)
+
# Cancel installation by deleting the installed files.
uninstall: lang.uninstall
-rm -rf $(DESTDIR)$(libsubdir)
diff --git a/gcc/config.in b/gcc/config.in
index 01fb18dbbb5a..ab51b9aef898 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -2042,6 +2042,12 @@
#endif
+/* Define if using LLVM tools. */
+#ifndef USED_FOR_TARGET
+#undef LLVM_TOOLS_WRAPPER
+#endif
+
+
/* Define to the name of the LTO plugin DSO that must be passed to the
linker's -plugin=LIB option. */
#ifndef USED_FOR_TARGET
diff --git a/gcc/configure b/gcc/configure
index 5381e107bce7..06f46c4bd6b0 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -22994,6 +22994,21 @@ else
$as_echo "$gcc_cv_otool" >&6; }
fi
+# Do we need the llvm-tools-wrapper?
+#TODO Make this a feature test, or is that good enough?
+case $target in
+ amdgcn*)
+
+$as_echo "#define LLVM_TOOLS_WRAPPER 1" >>confdefs.h
+
+ #TODO Does this need any 'Makefile.in' support for stating regeneration dependencies?
+ #TODO This is modelled after the 'exec-tool.in' stuff.
+ ac_config_files="$ac_config_files llvm-tools-wrapper:llvm-tools-wrapper.in"
+
+ ;;
+esac
+
+
# Figure out what assembler alignment features are present.
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler flags" >&5
$as_echo_n "checking assembler flags... " >&6; }
@@ -31460,6 +31475,7 @@ do
"as") CONFIG_FILES="$CONFIG_FILES as:exec-tool.in" ;;
"collect-ld") CONFIG_FILES="$CONFIG_FILES collect-ld:exec-tool.in" ;;
"nm") CONFIG_FILES="$CONFIG_FILES nm:exec-tool.in" ;;
+ "llvm-tools-wrapper") CONFIG_FILES="$CONFIG_FILES llvm-tools-wrapper:llvm-tools-wrapper.in" ;;
"clearcap.map") CONFIG_LINKS="$CONFIG_LINKS clearcap.map:${srcdir}/config/$clearcap_map" ;;
"$all_outputs") CONFIG_FILES="$CONFIG_FILES $all_outputs" ;;
"default") CONFIG_COMMANDS="$CONFIG_COMMANDS default" ;;
@@ -32094,6 +32110,7 @@ $as_echo "$as_me: executing $ac_file commands" >&6;}
"as":F) chmod +x as ;;
"collect-ld":F) chmod +x collect-ld ;;
"nm":F) chmod +x nm ;;
+ "llvm-tools-wrapper":F) chmod +x llvm-tools-wrapper ;;
"default":C)
case ${CONFIG_HEADERS} in
*auto-host.h:config.in*)
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 0d6230e0ca1b..0f3fcfefd4fc 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -2685,6 +2685,18 @@ else
AC_MSG_RESULT($gcc_cv_otool)
fi
+# Do we need the llvm-tools-wrapper?
+#TODO Make this a feature test, or is that good enough?
+case $target in
+ amdgcn*)
+ AC_DEFINE(LLVM_TOOLS_WRAPPER, 1, [Define if using LLVM tools.])
+ #TODO Does this need any 'Makefile.in' support for stating regeneration dependencies?
+ #TODO This is modelled after the 'exec-tool.in' stuff.
+ AC_CONFIG_FILES(llvm-tools-wrapper:llvm-tools-wrapper.in, [chmod +x llvm-tools-wrapper])
+ ;;
+esac
+
+
# Figure out what assembler alignment features are present.
gcc_GAS_CHECK_FEATURE([.balign and .p2align], gcc_cv_as_balign_and_p2align,
[2,6,0],,
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index eef1432147ce..d6057656c6ef 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -2333,9 +2333,6 @@ Target uses GNU @command{ld}.
Target keeps null pointer checks, either due to the use of
@option{-fno-delete-null-pointer-checks} or hardwired into the target.
-@item llvm_binutils
-Target is using an LLVM assembler and/or linker, instead of GNU Binutils.
-
@item lto
Compiler has been configured to support link-time optimization (LTO).
@@ -2363,9 +2360,6 @@ Target supports the @code{noinit} variable attribute.
@item nonpic
Target does not generate PIC by default.
-@item offload_gcn
-Target has been configured for OpenACC/OpenMP offloading on AMD GCN.
-
@item pie_enabled
Target generates PIE by default.
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 9f790db0daf4..eb0fb836ffc0 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3038,11 +3038,18 @@ execute (void)
gcc_assert (!processing_spec_function);
+ /* TODO Why is this wrapping here done "early", instead of "late", as done
+ for the 'valgrind' wrapping? */
if (wrapper_string)
{
string = find_a_file (&exec_prefixes,
argbuf[0], X_OK, false);
if (string)
+ /* This overwrites 'argbuf[0]', thus later 'commands[0].prog' is set to
+ the resolved 'string' instead of the original 'argbuf[0]'. This
+ means that any special handling for 'argbuf[0]' can no longer be
+ expected to work (such as when 'find_a_file' compares this to 'as'
+ or 'ld'). */
argbuf[0] = string;
insert_wrapper (wrapper_string);
}
@@ -3064,6 +3071,7 @@ execute (void)
commands[0].prog = argbuf[0]; /* first command. */
commands[0].argv = argbuf.address ();
+ /* TODO For 'wrapper_string', this has already been done above. */
if (!wrapper_string)
{
string = find_a_file (&exec_prefixes, commands[0].prog, X_OK, false);
@@ -3164,32 +3172,100 @@ execute (void)
#endif /* DEBUG */
}
-#ifdef ENABLE_VALGRIND_CHECKING
- /* Run the each command through valgrind. To simplify prepending the
- path to valgrind and the option "-q" (for quiet operation unless
- something triggers), we allocate a separate argv array. */
-
+ /* Possibly run each command through a wrapper. */
+ //TODO Why is this done after '-v' handling?
for (i = 0; i < n_commands; i++)
{
- const char **argv;
- int argc;
- int j;
-
- for (argc = 0; commands[i].argv[argc] != NULL; argc++)
- ;
+ size_t argc_wrap_llvm_tools_wrapper = 0;
+#ifdef LLVM_TOOLS_WRAPPER //TODO Conditional whether assembler (?TODO), linker (even separately?) are LLVM tools.
+ //TODO For offloading compilation (I haven't tried native yet), something isn't working: despite passing in '-fdiagnostics-color=never', the 'lld' wrapper invocation is still done with '-fdiagnostics-color=always' if printing to a terminal -- but does turn into '-fdiagnostics-color=never' if not printing to a terminal.
+ //TODO Does that mean that 'diagnostic_color_init' is only doing the 'DIAGNOSTICS_COLOR_DEFAULT' thing, but not paying attention to the command-line argument?
+ //TODO Per PR69707, that is supposed to be working?
+ //TODO Is that maybe a problem in 'mkoffload'? (..., and if yes, just amdgcn, or all?)
+ const char *llvm_tools_wrapper_color_arg = NULL;
+#if 0 //TODO Do this for 'as', too?
+ if (!strcmp (commands[i].prog, "as"))
+ {
+ /* That's 'llvm-mc'. */
+ argc_wrap_llvm_tools_wrapper = 1;
+ if (pp_show_color (global_dc->printer))
+ {
+ argc_wrap_llvm_tools_wrapper += 1;
+ llvm_tools_wrapper_color_arg = "--color";
+ }
+ else
+ ; /* There doesn't seem to be a command-line flag to force-disable
+ this, so let's hope the default does the right thing. */
+ }
+ else
+#endif
+ if (!strcmp (commands[i].prog, linker_name_spec))
+ {
+ /* That's 'lld'. */
+ argc_wrap_llvm_tools_wrapper = 1;
+ if (pp_show_color (global_dc->printer))
+ {
+ argc_wrap_llvm_tools_wrapper += 1;
+ llvm_tools_wrapper_color_arg = "--color-diagnostics=always";
+ }
+ else
+ {
+ argc_wrap_llvm_tools_wrapper += 1;
+ llvm_tools_wrapper_color_arg = "--color-diagnostics=never";
+ }
+ }
+#endif /* LLVM_TOOLS_WRAPPER */
+ size_t argc_wrap_valgrind = 0;
+#ifdef ENABLE_VALGRIND_CHECKING
+ argc_wrap_valgrind = 2;
+#endif
+ size_t argc_extra = argc_wrap_llvm_tools_wrapper + argc_wrap_valgrind;
+ if (argc_extra > 0)
+ {
+ /* One extra for the terminator. */
+ argc_extra += 1;
- argv = XALLOCAVEC (const char *, argc + 3);
+ size_t argc;
+ for (argc = 0; commands[i].argv[argc] != NULL; argc++)
+ ;
- argv[0] = VALGRIND_PATH;
- argv[1] = "-q";
- for (j = 2; j < argc + 2; j++)
- argv[j] = commands[i].argv[j - 2];
- argv[j] = NULL;
+ const char **argv = XALLOCAVEC (const char *, argc_extra + argc);
+ //TODO Where does this get deallocated?
- commands[i].argv = argv;
- commands[i].prog = argv[0];
- }
+ size_t j = 0;
+ if (argc_wrap_llvm_tools_wrapper)
+ {
+ //TODO This uses 'find_a_file' to locate this in the build directory. This supposedly should also work for the install tree. 'find_a_file' should be doing/supporting all that; this is modelled similar to 'lto-wrapper'.
+ argv[j++] = find_a_file (&exec_prefixes, "llvm-tools-wrapper", X_OK, false);
+ }
+ if (argc_wrap_valgrind)
+ {
+#ifdef ENABLE_VALGRIND_CHECKING
+ /* Run the each command through valgrind. */
+ argv[j++] = VALGRIND_PATH;
+ /* Request quiet operation unless something triggers. */
+ argv[j++] = "-q";
+#else
+ gcc_unreachable ();
#endif
+ }
+ /* Now the actual executable. */
+ argv[j++] = commands[i].argv[0];
+#ifdef LLVM_TOOLS_WRAPPER
+ /* Now any LLVM tools color arguments. */
+ if (llvm_tools_wrapper_color_arg)
+ argv[j++] = llvm_tools_wrapper_color_arg;
+#endif /* LLVM_TOOLS_WRAPPER */
+ /* Now all other arguments. */
+ for (size_t arg = 1; arg < argc; ++arg)
+ argv[j++] = commands[i].argv[arg];
+ argv[j++] = NULL;
+ gcc_checking_assert (j == argc + argc_extra);
+
+ commands[i].argv = argv;
+ commands[i].prog = argv[0];
+ }
+ }
/* Run each piped subprocess. */
diff --git a/gcc/llvm-tools-wrapper.in b/gcc/llvm-tools-wrapper.in
new file mode 100755
index 000000000000..a9233209082a
--- /dev/null
+++ b/gcc/llvm-tools-wrapper.in
@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+
+# This is a simple wrapper that invokes '$@', and filters 'stderr' such as to
+# only print non-empty lines.
+
+# Doing this, the invoked program may notice that it's not printing to a
+# terminal, and may change its behavior, say, to disable color output. If that
+# is a concern, it has to be addressed individually.
+
+# This implementation depends on the GNU Bash, in particular its 'coproc'
+# command. We expect this wrapper only to be used on systems where such
+# dependencies are not a concern.
+
+set -e
+
+# Set up a coprocess: print non-empty lines to 'stderr'.
+coproc grep --line-buffered . >&2
+
+# Invoke '$@'.
+status=0
+"$@" 2>&"${COPROC[1]}" || status="$?"
+
+# Shut down the coprocess.
+# Apparently, per
+# <https://unix.stackexchange.com/questions/86270/how-do-you-use-the-command-coproc-in-various-shells>,
+# we have to use such an indirection ('coproc_fd_1') to support "bash versions
+# prior to 4.3".
+coproc_fd_1=${COPROC[1]}
+exec {coproc_fd_1}<&-
+# We intentionally do not 'wait' for '$COPROC_PID' here, as that sometimes
+# fails (perhaps if the coprocess has never been active, if there has been no
+# output at all?).
+wait
+
+exit "$status"
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index cccd3ce4742c..941fc72b5947 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -342,10 +342,6 @@ proc gcc-dg-test { prog do_what extra_tool_flags } {
# for all tests.
set allow_blank_lines 0
-if { [check_effective_target_llvm_binutils] } {
- set allow_blank_lines 2
-}
-
# A command for use by testcases to mark themselves as expecting
# blank lines in the output.
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index ca3895c22690..089ecfb68ad4 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -9514,14 +9514,6 @@ proc check_effective_target_offload_hsa { } {
} "-foffload=hsa" ]
}
-# Return 1 if the compiler has been configured with hsa offloading.
-
-proc check_effective_target_offload_gcn { } {
- return [check_no_compiler_messages offload_gcn assembly {
- int main () {return 0;}
- } "-foffload=amdgcn-unknown-amdhsa" ]
-}
-
# Return 1 if the target support -fprofile-update=atomic
proc check_effective_target_profile_update_atomic {} {
return [check_no_compiler_messages profile_update_atomic assembly {
@@ -9963,13 +9955,6 @@ foreach N {df} {
}]
}
-# Return 1 if this target uses an LLVM assembler and/or linker
-proc check_effective_target_llvm_binutils { } {
- return [check_cached_effective_target llvm_binutils {
- expr { [istarget amdgcn*-*-*]
- || [check_effective_target_offload_gcn] }}]
-}
-
# Return 1 if the compiler supports '-mfentry'.
proc check_effective_target_mfentry { } {
diff --git a/libgomp/testsuite/libgomp.c-c++-common/function-not-offloaded.c b/libgomp/testsuite/libgomp.c-c++-common/function-not-offloaded.c
index f01a64e72c07..9e59ef8864e7 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/function-not-offloaded.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/function-not-offloaded.c
@@ -1,6 +1,5 @@
/* { dg-do link } */
/* { dg-excess-errors "unresolved symbol foo, lto1, mkoffload and lto-wrapper fatal errors" { target offload_device_nonshared_as } } */
-/* { dg-allow-blank-lines-in-output 1 } */
/* { dg-additional-sources "function-not-offloaded-aux.c" } */
#pragma omp declare target
--
2.25.1
