Skip to content

Commit ed95b99

Browse files
committed
gdb/testsuite: address PR review feedback for HIP compiler migration
- Generalize the -x hip / .o mixing fix: pre-compile each source to its own .o at the top of gdb_compile, then recurse with an all-.o source list. Fixes "expected unqualified-id" errors when amdclang++ sees -x hip applied to .o inputs. - Pass --rocm-path explicitly in HIP flags (both lib/gdb.exp and boards/hip.exp) so amdclang++ can locate HIP headers/libs regardless of how it is invoked. - Drop -fdiagnostics-color skip for HIP; amdclang++ supports it. - Remove HIPCC_FOR_TARGET support; hipcc is incompatible with the new -x hip / --hip-link flags. - Fix stale comments and a spurious empty line.
1 parent b7a3a82 commit ed95b99

2 files changed

Lines changed: 69 additions & 51 deletions

File tree

gdb/testsuite/boards/hip.exp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,10 @@ proc gdb_compile {source dest type options} {
302302
# '-mllvm -amdgpu-spill-cfi-saved-regs'
303303
# [-Wunused-command-line-argument]
304304
#
305+
# Pass --rocm-path explicitly so amdclang++ can locate the HIP
306+
# headers and device libraries when it's invoked from the board
307+
# via PATH rather than an absolute path. rocm_path comes from
308+
# rocm.exp and defaults to /opt/rocm if ROCM_PATH is not set.
305309
global rocm_path
306310
set hip_options "early_flags=\
307311
--rocm-path=$rocm_path\

gdb/testsuite/lib/gdb.exp

Lines changed: 65 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6248,6 +6248,44 @@ proc gdb_compile {source dest type options} {
62486248
}
62496249
}
62506250

6251+
# For HIP executables, amdclang++ needs -x hip to treat .cpp/.c as
6252+
# HIP source, but -x hip applied to a .o file makes amdclang++ try
6253+
# to parse the object as HIP source, which fails. Since gdb_compile
6254+
# may later inject additional .o inputs (e.g. set_unbuffered_mode.o),
6255+
# we cannot safely mix any source files with any .o inputs in a
6256+
# single amdclang++ invocation. To avoid that, pre-compile each
6257+
# source file to its own .o first (with -x hip), then recurse with
6258+
# an all-.o source list. The recursive call will not add -x hip
6259+
# and the final link will work correctly.
6260+
if {[lsearch -exact $options hip] != -1
6261+
&& $type eq "executable"
6262+
&& [lsearch -exact $options getting_compiler_info] == -1} {
6263+
set has_source 0
6264+
foreach src $source {
6265+
set ext [file extension $src]
6266+
if {$ext ne ".o" && $ext ne ".a"} {
6267+
set has_source 1
6268+
break
6269+
}
6270+
}
6271+
if {$has_source} {
6272+
set new_source {}
6273+
foreach src $source {
6274+
set ext [file extension $src]
6275+
if {$ext eq ".o" || $ext eq ".a"} {
6276+
lappend new_source $src
6277+
} else {
6278+
set out [standard_temp_file [file tail $src].hip.o]
6279+
set res [gdb_compile $src $out object $options]
6280+
if {$res ne ""} {
6281+
return $res
6282+
}
6283+
lappend new_source $out
6284+
}
6285+
}
6286+
return [gdb_compile $new_source $dest $type $options]
6287+
}
6288+
}
62516289

62526290
set outdir [file dirname $dest]
62536291

@@ -6559,11 +6597,16 @@ proc gdb_compile {source dest type options} {
65596597
#
65606598
# amdclang++ needs -x hip to treat .cpp/.c as HIP source when
65616599
# compiling, and --hip-link to link the HIP runtime when
6562-
# linking. When linking only .o files, -x hip must NOT be
6563-
# passed, otherwise amdclang++ tries to parse them as HIP
6564-
# source. Add -O0 as default optimization. If "optimize"
6565-
# is also requested, another -O flag (e.g. -O2) will be added
6566-
# to the flags, overriding -O0.
6600+
# linking. -x hip must NOT be passed when any .o input is
6601+
# present, otherwise amdclang++ tries to parse the .o file
6602+
# as HIP source. The executable-linking path above pre-
6603+
# compiles all sources to .o first, so by the time we get
6604+
# here with $type == "executable" all inputs are .o files.
6605+
# For the $type == "object" path, $source is always a single
6606+
# source file, so -x hip is safe. Add -O0 as default
6607+
# optimization. If "optimize" is also requested, another
6608+
# -O flag (e.g. -O2) will be added to the flags, overriding
6609+
# -O0.
65676610
set has_source_files 0
65686611
foreach src $source {
65696612
set ext [file extension $src]
@@ -6582,6 +6625,15 @@ proc gdb_compile {source dest type options} {
65826625
if {$type eq "executable"} {
65836626
set hip_early_flags "--hip-link $hip_early_flags"
65846627
}
6628+
# Pass --rocm-path explicitly so amdclang++ can locate the HIP
6629+
# headers and device libraries regardless of how it was invoked
6630+
# (full path vs bare name on PATH) or whether ROCM_PATH is set
6631+
# in the environment. rocm_path comes from rocm.exp and
6632+
# defaults to /opt/rocm if ROCM_PATH is not set.
6633+
global rocm_path
6634+
if {[info exists rocm_path] && $rocm_path ne ""} {
6635+
set hip_early_flags "--rocm-path=$rocm_path $hip_early_flags"
6636+
}
65856637
lappend new_options "early_flags=$hip_early_flags"
65866638

65876639
if {[istarget "*-*-mingw*"]} {
@@ -6807,52 +6859,14 @@ proc gdb_compile {source dest type options} {
68076859
lappend options \
68086860
"ldflags=$gdb_saved_set_unbuffered_mode_obj($unbuf_obj_lang)"
68096861
} else {
6810-
# For HIP, amdclang++ needs -x hip to treat
6811-
# sources as HIP, but -x hip applied to a .o
6812-
# file makes amdclang++ try to parse the object
6813-
# as HIP source, which fails. To avoid mixing
6814-
# sources and the unbuffered .o in a single
6815-
# amdclang++ invocation, pre-compile each source
6816-
# file to its own .o first (with -x hip), then
6817-
# replace the source list with the resulting
6818-
# .o files plus the unbuffered .o. The final
6819-
# link step must use only .o inputs, so also
6820-
# strip -x hip from the accumulated early_flags
6821-
# so the link sees just --hip-link.
6822-
set new_source {}
6823-
foreach src $source {
6824-
set ext [file extension $src]
6825-
if {$ext eq ".o" || $ext eq ".a"} {
6826-
lappend new_source $src
6827-
} else {
6828-
set out \
6829-
[standard_temp_file [file tail $src].hip.o]
6830-
set res [gdb_compile $src $out object $options]
6831-
if {$res ne ""} {
6832-
return $res
6833-
}
6834-
lappend new_source $out
6835-
}
6836-
}
6837-
lappend new_source \
6838-
$gdb_saved_set_unbuffered_mode_obj($unbuf_obj_lang)
6839-
set source $new_source
6840-
6841-
# Strip -x hip from early_flags in options since
6842-
# we no longer have any source files in the
6843-
# final link invocation.
6844-
set stripped_options {}
6845-
foreach opt $options {
6846-
if {[regexp {^early_flags=(.*)} $opt -> val]} {
6847-
regsub -all {(^| )-x hip( |$)} \
6848-
$val { } val
6849-
set val [string trim $val]
6850-
lappend stripped_options "early_flags=$val"
6851-
} else {
6852-
lappend stripped_options $opt
6853-
}
6854-
}
6855-
set options $stripped_options
6862+
# For HIP, the generalized pre-compile step at
6863+
# the top of gdb_compile has already turned any
6864+
# source files into .o files. Prepending the
6865+
# unbuffered .o to the (all-.o) source list is
6866+
# safe -- no mixed source/.o invocation happens.
6867+
set source \
6868+
[linsert $source 0 \
6869+
$gdb_saved_set_unbuffered_mode_obj($unbuf_obj_lang)]
68566870
}
68576871
}
68586872
}

0 commit comments

Comments
 (0)