Skip to content

Commit 0f37734

Browse files
David SalinasDavid Salinas
authored andcommitted
Windows hipcc does not handle spaces in quoted arguments correctly
SWDEV-441419 - Windows hipcc does not handle spaces in quoted arguments correctly SWDEV-441980 - [Compiler_Stg][GFX][HIP][WIN] HIP Kernel build failures observed in Redshift,Blender,GL and Vulkan and some minor typo cleanup. Change-Id: Iee786ba2b7bfd64540aa85d8a58fd465213c95fb
1 parent 9066af2 commit 0f37734

File tree

3 files changed

+46
-31
lines changed

3 files changed

+46
-31
lines changed

amd/hipcc/src/hipBin_amd.h

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ class HipBinAmd : public HipBinBase {
7676
// non virtual functions
7777
const string& getHsaPath() const;
7878
const string& getRocclrHomePath() const;
79+
const bool isWindows() const;
7980
};
8081

8182
HipBinAmd::HipBinAmd() {
@@ -196,26 +197,25 @@ void HipBinAmd::initializeHipCXXFlags() {
196197

197198
// populates clang path.
198199
void HipBinAmd::constructCompilerPath() {
199-
string complierPath;
200+
string compilerPath;
200201
const EnvVariables& envVariables = getEnvVariables();
201202
if (envVariables.hipClangPathEnv_.empty()) {
202203
fs::path hipClangPath;
203-
const OsType& osInfo = getOSInfo();
204-
if (osInfo == windows) {
205-
complierPath = getHipPath();
206-
hipClangPath = complierPath;
204+
if (isWindows()) {
205+
compilerPath = getHipPath();
206+
hipClangPath = compilerPath;
207+
hipClangPath /= "bin";
207208
} else {
208-
complierPath = getRoccmPath();
209-
hipClangPath = complierPath;
209+
compilerPath = getRoccmPath();
210+
hipClangPath = compilerPath;
211+
hipClangPath /= "llvm/bin";
210212
}
211213

212-
hipClangPath /= "llvm/bin";
213-
214-
complierPath = hipClangPath.string();
214+
compilerPath = hipClangPath.string();
215215
} else {
216-
complierPath = envVariables.hipClangPathEnv_;
216+
compilerPath = envVariables.hipClangPathEnv_;
217217
}
218-
hipClangPath_ = complierPath;
218+
hipClangPath_ = compilerPath;
219219
}
220220

221221
// returns clang path.
@@ -224,10 +224,9 @@ const string& HipBinAmd::getCompilerPath() const {
224224
}
225225

226226
void HipBinAmd::printCompilerInfo() const {
227-
const OsType& os = getOSInfo();
228227
const string& hipClangPath = getCompilerPath();
229228
const string& hipPath = getHipPath();
230-
if (os == windows) {
229+
if (isWindows()) {
231230
string cmd = hipClangPath + "/clang++ --print-resource-dir";
232231
system(cmd.c_str()); // hipclang version
233232
cout << "llc-version :" << endl;
@@ -256,7 +255,7 @@ void HipBinAmd::printCompilerInfo() const {
256255
}
257256

258257
string HipBinAmd::getCompilerVersion() {
259-
string out, complierVersion;
258+
string out, compilerVersion;
260259
const string& hipClangPath = getCompilerPath();
261260
fs::path cmdAmd = hipClangPath;
262261
cmdAmd /= "clang++";
@@ -267,13 +266,13 @@ string HipBinAmd::getCompilerVersion() {
267266
if (m.size() > 1) {
268267
// get the index =1 match, 0=whole match we ignore
269268
std::ssub_match sub_match = m[1];
270-
complierVersion = sub_match.str();
269+
compilerVersion = sub_match.str();
271270
}
272271
}
273272
} else {
274273
std::cerr << "Hip Clang Compiler not found" << endl;
275274
}
276-
return complierVersion;
275+
return compilerVersion;
277276
}
278277

279278

@@ -293,8 +292,7 @@ string HipBinAmd::getCppConfig() {
293292
const string& hipPath = getHipPath();
294293
hipPathInclude = hipPath;
295294
hipPathInclude /= "include";
296-
const OsType& osInfo = getOSInfo();
297-
if (osInfo == windows) {
295+
if (isWindows()) {
298296
cppConfig += " -I" + hipPathInclude.string();
299297
cppConfigFs = cppConfig;
300298
cppConfigFs /= "/";
@@ -359,12 +357,19 @@ string HipBinAmd::getHipCC() const {
359357
string hipCC;
360358
const string& hipClangPath = getCompilerPath();
361359
fs::path compiler = hipClangPath;
362-
compiler /= "clang++";
360+
if (isWindows())
361+
compiler /= "clang.exe";
362+
else
363+
compiler /= "clang++";
364+
363365
if (!fs::exists(compiler)) {
364366
fs::path compiler = hipClangPath;
365367
compiler /= "clang";
366368
}
367369
hipCC = compiler.string();
370+
371+
if (isWindows()) // wrap hipcc (clang) command in escaped double quotes.
372+
hipCC = "\"" + hipCC + "\" ";
368373
return hipCC;
369374
}
370375

@@ -423,6 +428,10 @@ void HipBinAmd::printFull() {
423428
cout << endl;
424429
}
425430

431+
const bool HipBinAmd::isWindows() const {
432+
const OsType& osInfo = getOSInfo();
433+
return (osInfo == windows);
434+
}
426435

427436
void HipBinAmd::executeHipCCCmd(vector<string> argv) {
428437
if (argv.size() < 2) {
@@ -668,6 +677,9 @@ void HipBinAmd::executeHipCCCmd(vector<string> argv) {
668677
} else if (hasCXX || hasHIP) {
669678
needCXXFLAGS = 1;
670679
}
680+
if (isWindows())
681+
arg = "\"" + arg + "\"";
682+
671683
inputs.push_back(arg);
672684
// print "I: <$arg>\n";
673685
}
@@ -677,7 +689,7 @@ void HipBinAmd::executeHipCCCmd(vector<string> argv) {
677689
// Do the quoting here because sometimes the $arg is changed in the loop
678690
// Important to have all of '-Xlinker' in the set of unquoted characters.
679691
// Windows needs different quoting, ignore for now
680-
if (os != windows && escapeArg) {
692+
if (!isWindows() && escapeArg) {
681693
regex reg("[^-a-zA-Z0-9_=+,.\\/]");
682694
arg = regex_replace(arg, reg, "\\$&");
683695
}
@@ -885,12 +897,15 @@ void HipBinAmd::executeHipCCCmd(vector<string> argv) {
885897
cout << HIPLDFLAGS;
886898
}
887899
if (runCmd) {
900+
if (isWindows())
901+
CMD = "\"" + CMD + "\"";
902+
888903
SystemCmdOut sysOut;
889904
sysOut = hipBinUtilPtr_->exec(CMD.c_str(), true);
890905
string cmdOut = sysOut.out;
891906
int CMD_EXIT_CODE = sysOut.exitCode;
892907
if (CMD_EXIT_CODE !=0) {
893-
std::cerr << "failed to execute:" << CMD << std::endl;
908+
std::cerr << "failed to execute:" << CMD << std::endl;
894909
}
895910
exit(CMD_EXIT_CODE);
896911
} // end of runCmd section

amd/hipcc/src/hipBin_base.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -463,14 +463,14 @@ void HipBinBase::printUsage() const {
463463

464464
// compiler canRun or not
465465
bool HipBinBase::canRunCompiler(string exeName, string& cmdOut) {
466-
string complierName = exeName;
466+
string compilerName = exeName;
467467
string temp_dir = hipBinUtilPtr_->getTempDir();
468468
fs::path templateFs = temp_dir;
469469
templateFs /= "canRunXXXXXX";
470470
string tmpFileName = hipBinUtilPtr_->mktempFile(templateFs.string());
471-
complierName += " --version > " + tmpFileName + " 2>&1";
471+
compilerName += " --version > " + tmpFileName + " 2>&1";
472472
bool executable = false;
473-
if (system(const_cast<char*>(complierName.c_str()))) {
473+
if (system(const_cast<char*>(compilerName.c_str()))) {
474474
executable = false;
475475
} else {
476476
string myline;

amd/hipcc/src/hipBin_nvidia.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,16 +229,16 @@ string HipBinNvidia::getHipLibPath() const {
229229

230230
// gets nvcc compiler Path
231231
void HipBinNvidia::constructCompilerPath() {
232-
string complierPath;
232+
string compilerPath;
233233
const EnvVariables& envVariables = getEnvVariables();
234234
if (envVariables.cudaPathEnv_.empty()) {
235235
fs::path cudaPathfs;
236236
cudaPathfs = "/usr/local/cuda";
237-
complierPath = cudaPathfs.string();
237+
compilerPath = cudaPathfs.string();
238238
} else {
239-
complierPath = envVariables.cudaPathEnv_;
239+
compilerPath = envVariables.cudaPathEnv_;
240240
}
241-
cudaPath_ = complierPath;
241+
cudaPath_ = compilerPath;
242242
}
243243

244244

@@ -259,13 +259,13 @@ void HipBinNvidia::printCompilerInfo() const {
259259

260260
// returns nvcc version
261261
string HipBinNvidia::getCompilerVersion() {
262-
string complierVersion, cmd;
262+
string compilerVersion, cmd;
263263
fs::path nvcc;
264264
nvcc = getCompilerPath();
265265
nvcc /= "bin/nvcc";
266266
cmd = nvcc.string() + " --version";
267267
system(cmd.c_str());
268-
return complierVersion;
268+
return compilerVersion;
269269
}
270270

271271
// returns nvidia platform

0 commit comments

Comments
 (0)