subject:"\[PATCH\] D73299\: \[HIP\] Fix environment variable HIP_DEVICE_LIB

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

2020-01-28 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

In D73299#1845162 , @thakis wrote:

> This breaks check-clang on Windows: http://45.33.8.238/win/6843/step_7.txt
>
> You probably want "env FOO=bar" instead of "FOO=bar" in the test file.


Fixed. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73299/new/

https://reviews.llvm.org/D73299



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

2020-01-28 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

This breaks check-clang on Windows: http://45.33.8.238/win/6843/step_7.txt

You probably want "env FOO=bar" instead of "FOO=bar" in the test file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73299/new/

https://reviews.llvm.org/D73299



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

2020-01-28 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7e415f37f38: [HIP] Fix environment variable 
HIP_DEVICE_LIB_PATH (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73299/new/

https://reviews.llvm.org/D73299

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-device-libs.hip


Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -19,6 +19,13 @@
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
+// Test environment variable HIP_DEVICE_LIB_PATH
+
+// RUN: HIP_DEVICE_LIB_PATH=%S/Inputs/hip_dev_lib \
+// RUN:   %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=COM
 
 // COM: {{"[^"]*clang[^"]*"}}
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.amdgcn.bc"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -327,7 +327,7 @@
DriverArgs.getAllArgValues(options::OPT_hip_device_lib_path_EQ))
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
-  addDirectoryList(DriverArgs, LibraryPaths, "-L", "HIP_DEVICE_LIB_PATH");
+  addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
 
   llvm::SmallVector BCLibs;
 
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -94,6 +94,11 @@
 
 bool isUseSeparateSections(const llvm::Triple );
 
+/// \p EnvVar is split by system delimiter for environment variables.
+/// If \p ArgName is "-I", "-L", or an empty string, each entry from \p EnvVar
+/// is prefixed by \p ArgName then added to \p Args. Otherwise, for each
+/// entry of \p EnvVar, \p ArgName is added to \p Args first, then the entry
+/// itself is added.
 void addDirectoryList(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList , const char *ArgName,
   const char *EnvVar);
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -91,7 +91,7 @@
 return; // Nothing to do.
 
   StringRef Name(ArgName);
-  if (Name.equals("-I") || Name.equals("-L"))
+  if (Name.equals("-I") || Name.equals("-L") || Name.empty())
 CombinedArg = true;
 
   StringRef Dirs(DirList);


Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -19,6 +19,13 @@
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
+// Test environment variable HIP_DEVICE_LIB_PATH
+
+// RUN: HIP_DEVICE_LIB_PATH=%S/Inputs/hip_dev_lib \
+// RUN:   %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=COM
 
 // COM: {{"[^"]*clang[^"]*"}}
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.amdgcn.bc"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -327,7 +327,7 @@
DriverArgs.getAllArgValues(options::OPT_hip_device_lib_path_EQ))
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
-  addDirectoryList(DriverArgs, LibraryPaths, "-L", "HIP_DEVICE_LIB_PATH");
+  addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
 
   llvm::SmallVector BCLibs;
 
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -94,6 +94,11 @@
 
 bool isUseSeparateSections(const llvm::Triple );
 
+/// \p EnvVar is split by system delimiter for environment variables.
+/// If \p ArgName is "-I", "-L", or an empty string, each entry from \p EnvVar
+/// is prefixed by \p ArgName then added to \p Args. Otherwise, for each
+/// entry of \p EnvVar, \p ArgName is added to \p Args first, then the entry
+/// itself is added.
 void addDirectoryList(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList , const char *ArgName,

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

2020-01-23 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 240074.
yaxunl edited the summary of this revision.
yaxunl added a comment.

Revised by Artem's comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73299/new/

https://reviews.llvm.org/D73299

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-device-libs.hip


Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -19,6 +19,13 @@
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
+// Test environment variable HIP_DEVICE_LIB_PATH
+
+// RUN: HIP_DEVICE_LIB_PATH=%S/Inputs/hip_dev_lib \
+// RUN:   %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=COM
 
 // COM: {{"[^"]*clang[^"]*"}}
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.amdgcn.bc"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -327,7 +327,7 @@
DriverArgs.getAllArgValues(options::OPT_hip_device_lib_path_EQ))
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
-  addDirectoryList(DriverArgs, LibraryPaths, "-L", "HIP_DEVICE_LIB_PATH");
+  addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
 
   llvm::SmallVector BCLibs;
 
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -94,6 +94,11 @@
 
 bool isUseSeparateSections(const llvm::Triple );
 
+/// \p EnvVar is split by system delimiter for environment variables.
+/// If \p ArgName is "-I", "-L", or an empty string, each entry from \p EnvVar
+/// is prefixed by \p ArgName then added to \p Args. Otherwise, for each
+/// entry of \p EnvVar, \p ArgName is added to \p Args first, then the entry
+/// itself is added.
 void addDirectoryList(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList , const char *ArgName,
   const char *EnvVar);
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -91,7 +91,7 @@
 return; // Nothing to do.
 
   StringRef Name(ArgName);
-  if (Name.equals("-I") || Name.equals("-L"))
+  if (Name.equals("-I") || Name.equals("-L") || Name.empty())
 CombinedArg = true;
 
   StringRef Dirs(DirList);


Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -19,6 +19,13 @@
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
+// Test environment variable HIP_DEVICE_LIB_PATH
+
+// RUN: HIP_DEVICE_LIB_PATH=%S/Inputs/hip_dev_lib \
+// RUN:   %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=COM
 
 // COM: {{"[^"]*clang[^"]*"}}
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.amdgcn.bc"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -327,7 +327,7 @@
DriverArgs.getAllArgValues(options::OPT_hip_device_lib_path_EQ))
 LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
-  addDirectoryList(DriverArgs, LibraryPaths, "-L", "HIP_DEVICE_LIB_PATH");
+  addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
 
   llvm::SmallVector BCLibs;
 
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -94,6 +94,11 @@
 
 bool isUseSeparateSections(const llvm::Triple );
 
+/// \p EnvVar is split by system delimiter for environment variables.
+/// If \p ArgName is "-I", "-L", or an empty string, each entry from \p EnvVar
+/// is prefixed by \p ArgName then added to \p Args. Otherwise, for each
+/// entry of \p EnvVar, \p ArgName is added to \p Args first, then the entry
+/// itself is added.
 void addDirectoryList(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList , const char *ArgName,
   const char *EnvVar);
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

2020-01-23 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

> If user sets HIP_DEVICE_LIB_PATH, this line in HIP.cpp
>  addDirectoryList(DriverArgs, LibraryPaths, "-L", "HIP_DEVICE_LIB_PATH");
>  adds -L to LibraryPaths. -L is
>  needed since otherwise addDirectoryList will insert an extra empty string.
>  However, the -L must be stripped before checking for existence in addBCLib.

The fact that "-I" and "-L" are prepended to the paths is an implementation 
detail.
IMO it would be better to fix addDirectoryList() so it can deal with empty 
prefixes, and document it in the CommonArgs.h.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73299/new/

https://reviews.llvm.org/D73299



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

2020-01-23 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added a reviewer: tra.

If user sets HIP_DEVICE_LIB_PATH, this line in HIP.cpp
addDirectoryList(DriverArgs, LibraryPaths, "-L", "HIP_DEVICE_LIB_PATH");
adds `-L` to LibraryPaths. `-L` is
needed since otherwise addDirectoryList will insert an extra empty string.
However, the `-L` must be stripped before checking for existence in addBCLib.

Patch by Greg Rodgers.


https://reviews.llvm.org/D73299

Files:
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-device-libs.hip


Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -19,6 +19,13 @@
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
+// Test environment variable HIP_DEVICE_LIB_PATH
+
+// RUN: HIP_DEVICE_LIB_PATH=%S/Inputs/hip_dev_lib \
+// RUN:   %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=COM
 
 // COM: {{"[^"]*clang[^"]*"}}
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.amdgcn.bc"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -39,6 +39,8 @@
 SmallString<128> Path(LibraryPath);
 llvm::sys::path::append(Path, BCName);
 FullName = Path;
+if (FullName.startswith("-L"))
+  FullName = FullName.substr(2);
 if (llvm::sys::fs::exists(FullName)) {
   CmdArgs.push_back("-mlink-builtin-bitcode");
   CmdArgs.push_back(Args.MakeArgString(FullName));


Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -19,6 +19,13 @@
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
+// Test environment variable HIP_DEVICE_LIB_PATH
+
+// RUN: HIP_DEVICE_LIB_PATH=%S/Inputs/hip_dev_lib \
+// RUN:   %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=COM
 
 // COM: {{"[^"]*clang[^"]*"}}
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.amdgcn.bc"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -39,6 +39,8 @@
 SmallString<128> Path(LibraryPath);
 llvm::sys::path::append(Path, BCName);
 FullName = Path;
+if (FullName.startswith("-L"))
+  FullName = FullName.substr(2);
 if (llvm::sys::fs::exists(FullName)) {
   CmdArgs.push_back("-mlink-builtin-bitcode");
   CmdArgs.push_back(Args.MakeArgString(FullName));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

6 matches

Site Navigation

Mail list logo

Footer information