Hi!

Now that I'm looking into enabling AMD GCN offloading in my builds, I
have a few concerns here.  ;-)

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).

>>      gcc/testsuite/

>>      * lib/gcc-dg.exp (gcc-dg-prune): Ignore blank lines from the LLVM
>>      linker.
>>      * lib/target-supports.exp (check_effective_target_llvm_binutils): New.

> This is fine.  It's a NOP for other targets, so feel free to commit when
> it's convenient for you.

See below, is it really "a NOP for other targets"?

| --- a/gcc/testsuite/lib/gcc-dg.exp
| +++ b/gcc/testsuite/lib/gcc-dg.exp
| @@ -361,7 +361,7 @@ proc gcc-dg-prune { system text } {
|
|      # Complain about blank lines in the output (PR other/69006)
|      global allow_blank_lines
| -    if { !$allow_blank_lines } {
| +    if { !$allow_blank_lines && ![check_effective_target_llvm_binutils]} {
|       set num_blank_lines [llength [regexp -all -inline "\n\n" $text]]
|       if { $num_blank_lines } {
|           global testname_with_flags

(That got re-worked a bit, per <https://gcc.gnu.org/PR88920>.)

| --- a/gcc/testsuite/lib/target-supports.exp
| +++ b/gcc/testsuite/lib/target-supports.exp
| @@ -9129,6 +9129,14 @@ proc check_effective_target_offload_hsa { } {
|      } "-foffload=hsa" ]
|  }
|
| +# Return 1 if the compiler has been configured with hsa offloading.

(Is this conceptually really appropriate to have here in
'gcc/testsuite/', given that all 'mkoffload'-based offloading testing
happens in libgomp only?)  (And, 'hsa' copy'n'pasto should be 'amdgcn'?)

| +
| +proc check_effective_target_offload_gcn { } {
| +    return [check_no_compiler_messages offload_gcn assembly {
| +     int main () {return 0;}
| +    } "-foffload=amdgcn-unknown-amdhsa" ]
| +}

This is too specific: "amdgcn-unknown-amdhsa" is often spelled just
"amdgcn-amdhsa", for example.

Our current '-foffload' syntax is not amenable to such variations; we
really need to re-work that.  (That's also one of the reasons why
<https://gcc.gnu.org/PR67300> "-foffload* undocumented" is still open,
and hasn't seen any further work: the '-foffload' as we've currently got
it implemented is somewhat useful, yes, but needs to be improved to be
really useful for users -- as well as for ourselves, as we're seeing
here.)  For example, consider you'd like to do offloading compilation to
include code for two different AMD GCN ISAs (fat binaries).  Or, once
that is supported for OpenACC, "offloading" to several different
multicore CPU ISAs/variations.  Can't specify such things given the
current syntax; we need some kind of abstraction from offload target.
(But that's a separate discussion, of course.)

Anyway, per the current code here, if instead of amdgcn-unknown-amdhsa I
configure GCC for amdgcn-amdhsa offload target, this test will not work.

| +# Return 1 if this target uses an LLVM assembler and/or linker
| +proc check_effective_target_llvm_binutils { } {
| +    return [expr { [istarget amdgcn*-*-*]
| +                || [check_effective_target_offload_gcn] } ]
| +}

Unless I'm understanding something wrong, this (second condition here)
means that all the checking added for <https://gcc.gnu.org/PR69006>
"Extraneous newline emitted between error messages in GCC 6" will be void
as soon as GCC is configured for AMD GCN offloading.  (This
effective-target here doesn't just apply to AMD GCN offloading
compliation, but to *all* standard GCC testsuite checking, doesn't it?)
That seems problematic to me: conceptually, but also in practice, given
that more and more users (for example, major GNU/Linux distributions) are
enabling GCC offloading compilation.


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).

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/'?  ("Just" need to figure out how to
integrate that into the GCC build process, top-level build system.)


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

Reply via email to