Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

Fix for rsp files on Windows. #100

Merged
merged 2 commits into from
Aug 14, 2023
Merged

Fix for rsp files on Windows. #100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
928e0a8
Fix for rsp files on Windows. File names starting with @ can not be e…
cjatin Jun 29, 2023
5e94da1
Merge branch 'develop' into rsp
cjatin Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bin/hipcc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ $USE_PERL_SCRIPT //= 1; # use defined-or assignment operator. Use env var, but

my $isWindows = ($^O eq 'MSWin32' or $^O eq 'msys');
# escapes args with quotes SWDEV-341955
# do not escape @ for rsp files on windows
foreach $arg (@ARGV) {
if ($isWindows) {
$arg =~ s/[^-a-zA-Z0-9_=+,.:\/\\ ]/\\$&/g;
$arg =~ s/[^-a-zA-Z0-9_=+,.:@\/\\ ]/\\$&/g;
}
}

Expand Down
13 changes: 11 additions & 2 deletions bin/hipcc.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,15 @@ BEGIN
$hsacoVersion = $arg;
$swallowArg = 1;
}
if($arg =~ /\.rsp$/) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example response file that this patch is trying to fix? What is the error without this patch?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error that we see is one hip-tests, GraphTests have grown in size and the final linkage line is too big on windows.

For now we have fixed it via a hack: ROCm/hip-tests@fe9515a basically split it in two exe till we have a fix in place.

To fix this we need to enable rsp files inside hip-tests CMake (done via: ROCm/hip-tests#360 ) but before that hipcc needs to escape rsp files and add proper encoding for windows.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: we got same error for memory tests as well: ROCm/hip-tests#362

$pre_rsp = " ";
if($isWindows) {
$pre_rsp = " --rsp-quoting=windows ";
}
$HIPLDFLAGS .= $pre_rsp.$arg;
$HIPCXXFLAGS .= $pre_rsp.$arg;
$swallowArg = 1;
}

# nvcc does not handle standard compiler options properly
# This can prevent hipcc being used as standard CXX/C Compiler
Expand Down Expand Up @@ -467,10 +476,10 @@ BEGIN
# Do the quoting here because sometimes the $arg is changed in the loop
# Important to have all of '-Xlinker' in the set of unquoted characters.
if (not $isWindows and $escapeArg) {
$arg =~ s/[^-a-zA-Z0-9_=+,.\/]/\\$&/g;
$arg =~ s/[^-a-zA-Z0-9_=+,.@\/]/\\$&/g;
}
if ($isWindows and $escapeArg) {
$arg =~ s/[^-a-zA-Z0-9_=+,.:\/\\]/\\$&/g;
$arg =~ s/[^-a-zA-Z0-9_=+,.:@\/\\]/\\$&/g;
}
$toolArgs .= " $arg" unless $swallowArg;
$prevArg = $arg;
Expand Down