| Message ID | 58748b9db7927f39a5740f42e7c30386d61a4080.1536144068.git.ams@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | AMD GCN Port | expand |
On 9/5/18 5:52 AM, ams@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, so the "blank line(s) in output" tests are inappropriate (and very > noisy). > > The LLVM tools also have different command line options, so it's not possible > to autodetect object formats in the same way. > > This patch addresses both issues. > > 2018-09-05 Andrew Stubbs <ams@codesourcery.com> > > gcc/testsuite/ > * lib/file-format.exp (gcc_target_object_format): Handle AMD GCN. > * 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. jeff
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 <law@redhat.com> wrote: > On 9/5/18 5:52 AM, ams@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
Hi! On 2020-03-23T16:29:40+0100, I wrote: > On 2018-09-14T10:18:12-0600, Jeff Law <law@redhat.com> wrote: >> On 9/5/18 5:52 AM, ams@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
diff --git a/gcc/testsuite/lib/file-format.exp b/gcc/testsuite/lib/file-format.exp index 5c47246..c595fe2 100644 --- a/gcc/testsuite/lib/file-format.exp +++ b/gcc/testsuite/lib/file-format.exp @@ -41,6 +41,9 @@ proc gcc_target_object_format { } { } elseif { [istarget *-*-aix*] } { # AIX doesn't necessarily have objdump, so hand-code it. set gcc_target_object_format_saved coff + } elseif { [istarget *-*-amdhsa*] } { + # AMD GCN uses LLVM objdump which is not CLI-compatible + set gcc_target_object_format_saved elf } else { set objdump_name [find_binutils_prog objdump] set open_file [open objfmtst.c w] diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp index f5e6bef..7df348e 100644 --- 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 diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 61442bd..1e627fa 100644 --- 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. + +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 { @@ -9427,3 +9435,9 @@ proc check_effective_target_cet { } { } } "-O2" ] } + +# 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] } ] +}