[PATCH] D88730: [HIP] Fix default output file for -E
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b551b79d3bb: [HIP] Fix default output file for -E (authored
by yaxunl).
Herald added a project: clang.
Changed prior to commit:
https://reviews.llvm.org/D88730?vs=295799=296088#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88730/new/
https://reviews.llvm.org/D88730
Files:
clang/lib/Driver/Driver.cpp
clang/test/Driver/hip-output-file-name.hip
Index: clang/test/Driver/hip-output-file-name.hip
===
--- clang/test/Driver/hip-output-file-name.hip
+++ clang/test/Driver/hip-output-file-name.hip
@@ -7,3 +7,45 @@
// RUN: 2>&1 | FileCheck %s
// CHECK: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-output-file-name.o"
+
+// Check -E default output is "-" (stdout).
+
+// RUN: %clang -### -E -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s
+
+// RUN: %clang -### -E -save-temps -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s
+
+// RUN: %clang -### -E --cuda-device-only -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=CLANG-DASH %s
+
+// RUN: %clang -### -E --cuda-host-only -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=CLANG-DASH %s
+
+// DASH: {{.*}}clang-offload-bundler{{.*}}"-outputs=-"
+// CLANG-DASH: {{.*}}clang{{.*}}"-o" "-"
+
+// Check -E with -o.
+
+// RUN: %clang -### -E -o test.cui -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=OUT %s
+
+// RUN: %clang -### -E -o test.cui -save-temps -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=OUT %s
+
+// RUN: %clang -### -E -o test.cui --cuda-device-only -target x86_64-linux-gnu
\
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=CLANG-OUT %s
+
+// RUN: %clang -### -E -o test.cui --cuda-host-only -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=CLANG-OUT %s
+
+// OUT: {{.*}}clang-offload-bundler{{.*}}"-outputs=test.cui"
+// CLANG-OUT: {{.*}}clang{{.*}}"-o" "test.cui"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4604,6 +4604,17 @@
return Args.MakeArgString(Filename.c_str());
}
+static bool HasPreprocessOutput(const Action ) {
+ if (isa(JA))
+return true;
+ if (isa(JA) && isa(JA.getInputs()[0]))
+return true;
+ if (isa(JA) &&
+ HasPreprocessOutput(*(JA.getInputs()[0])))
+return true;
+ return false;
+}
+
const char *Driver::GetNamedOutputPath(Compilation , const JobAction ,
const char *BaseInput,
StringRef BoundArch, bool AtTopLevel,
@@ -4629,8 +4640,9 @@
}
// Default to writing to stdout?
- if (AtTopLevel && !CCGenDiagnostics && isa(JA))
+ if (AtTopLevel && !CCGenDiagnostics && HasPreprocessOutput(JA)) {
return "-";
+ }
// Is this the assembly listing for /FA?
if (JA.getType() == types::TY_PP_Asm &&
Index: clang/test/Driver/hip-output-file-name.hip
===
--- clang/test/Driver/hip-output-file-name.hip
+++ clang/test/Driver/hip-output-file-name.hip
@@ -7,3 +7,45 @@
// RUN: 2>&1 | FileCheck %s
// CHECK: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-output-file-name.o"
+
+// Check -E default output is "-" (stdout).
+
+// RUN: %clang -### -E -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s
+
+// RUN: %clang -### -E -save-temps -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s
+
+// RUN: %clang -### -E --cuda-device-only -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=CLANG-DASH %s
+
+// RUN: %clang -### -E --cuda-host-only -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=CLANG-DASH %s
+
+// DASH: {{.*}}clang-offload-bundler{{.*}}"-outputs=-"
+// CLANG-DASH: {{.*}}clang{{.*}}"-o" "-"
+
+// Check -E with -o.
+
+// RUN: %clang -### -E -o test.cui -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck
[PATCH] D88730: [HIP] Fix default output file for -E
tra added a comment. LGTM. Comment at: clang/test/Driver/hip-output-file-name.hip:13-15 +// RUN: %clang -### -E -target x86_64-linux-gnu \ +// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \ +// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s yaxunl wrote: > tra wrote: > > What does it mean for `-E` to be used when we compile for host and multiple > > devices. I believe for CUDA clang errors out unless there's only one > > sub-compilation. What does HIP do when it's run with `-E -o -` ? > > > > Looks like CUDA (and, maybe HIP, too) has a bug there. `-E` will run > > preprocess on all subcompilations. `-E -o -` will error out claiming that > > you can't use `-o` for multiple output files, even though `-###` shows the > > same `-o -` in all subcompilations in both cases. > > > > > > > > > HIP will emit a clang-offload-bundler file in textual format. Clang is able > to consume this file by unbundling it. I think ccache uses it to generate a > hash to check if a file and its dependent header files have changed. Since > this bundled file is in text format, ccache is able to use it to generate a > hash which covers both host dependence and device dependence. That would explain why I sometimes get a screenfull of noise when I do something like that with HIP. :-/ My expectation of `-E [-o -]` is that I get the text of preprocessed source. In this regard, CUDA's concatenate all preprocessd outputs is a bit better than HIPs' "here's a binary blob with preprocessed output inside". This is a bit of a mess that's beyond the scope of this CL and should be dealt with separately. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88730/new/ https://reviews.llvm.org/D88730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88730: [HIP] Fix default output file for -E
yaxunl added inline comments. Comment at: clang/test/Driver/hip-output-file-name.hip:13-15 +// RUN: %clang -### -E -target x86_64-linux-gnu \ +// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \ +// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s tra wrote: > What does it mean for `-E` to be used when we compile for host and multiple > devices. I believe for CUDA clang errors out unless there's only one > sub-compilation. What does HIP do when it's run with `-E -o -` ? > > Looks like CUDA (and, maybe HIP, too) has a bug there. `-E` will run > preprocess on all subcompilations. `-E -o -` will error out claiming that you > can't use `-o` for multiple output files, even though `-###` shows the same > `-o -` in all subcompilations in both cases. > > > > HIP will emit a clang-offload-bundler file in textual format. Clang is able to consume this file by unbundling it. I think ccache uses it to generate a hash to check if a file and its dependent header files have changed. Since this bundled file is in text format, ccache is able to use it to generate a hash which covers both host dependence and device dependence. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88730/new/ https://reviews.llvm.org/D88730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88730: [HIP] Fix default output file for -E
tra accepted this revision. tra added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/hip-output-file-name.hip:13-15 +// RUN: %clang -### -E -target x86_64-linux-gnu \ +// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \ +// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s What does it mean for `-E` to be used when we compile for host and multiple devices. I believe for CUDA clang errors out unless there's only one sub-compilation. What does HIP do when it's run with `-E -o -` ? Looks like CUDA (and, maybe HIP, too) has a bug there. `-E` will run preprocess on all subcompilations. `-E -o -` will error out claiming that you can't use `-o` for multiple output files, even though `-###` shows the same `-o -` in all subcompilations in both cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88730/new/ https://reviews.llvm.org/D88730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88730: [HIP] Fix default output file for -E
yaxunl created this revision.
yaxunl added a reviewer: tra.
yaxunl requested review of this revision.
By convention the default output file for -E is "-" (stdout).
This is expected by tools like ccache, which uses output
of -E to determine if a file and its dependence has changed.
Currently clang does not use stdout as default output file for -E
for HIP, which causes ccache not working.
This patch fixes that.
https://reviews.llvm.org/D88730
Files:
clang/lib/Driver/Driver.cpp
clang/test/Driver/hip-output-file-name.hip
Index: clang/test/Driver/hip-output-file-name.hip
===
--- clang/test/Driver/hip-output-file-name.hip
+++ clang/test/Driver/hip-output-file-name.hip
@@ -7,3 +7,27 @@
// RUN: 2>&1 | FileCheck %s
// CHECK: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-output-file-name.o"
+
+// Check -E default output is "-" (stdout).
+
+// RUN: %clang -### -E -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s
+
+// RUN: %clang -### -E -save-temps -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s
+
+// DASH: {{.*}}clang-offload-bundler{{.*}}"-outputs=-"
+
+// Check -E with -o.
+
+// RUN: %clang -### -E -o test.cui -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=OUT %s
+
+// RUN: %clang -### -E -o test.cui -save-temps -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=OUT %s
+
+// OUT: {{.*}}clang-offload-bundler{{.*}}"-outputs=test.cui"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4604,6 +4604,17 @@
return Args.MakeArgString(Filename.c_str());
}
+static bool HasPreprocessOutput(const Action ) {
+ if (isa(JA))
+return true;
+ if (isa(JA) && isa(JA.getInputs()[0]))
+return true;
+ if (isa(JA) &&
+ HasPreprocessOutput(*(JA.getInputs()[0])))
+return true;
+ return false;
+}
+
const char *Driver::GetNamedOutputPath(Compilation , const JobAction ,
const char *BaseInput,
StringRef BoundArch, bool AtTopLevel,
@@ -4629,8 +4640,9 @@
}
// Default to writing to stdout?
- if (AtTopLevel && !CCGenDiagnostics && isa(JA))
+ if (AtTopLevel && !CCGenDiagnostics && HasPreprocessOutput(JA)) {
return "-";
+ }
// Is this the assembly listing for /FA?
if (JA.getType() == types::TY_PP_Asm &&
Index: clang/test/Driver/hip-output-file-name.hip
===
--- clang/test/Driver/hip-output-file-name.hip
+++ clang/test/Driver/hip-output-file-name.hip
@@ -7,3 +7,27 @@
// RUN: 2>&1 | FileCheck %s
// CHECK: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-output-file-name.o"
+
+// Check -E default output is "-" (stdout).
+
+// RUN: %clang -### -E -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s
+
+// RUN: %clang -### -E -save-temps -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=DASH %s
+
+// DASH: {{.*}}clang-offload-bundler{{.*}}"-outputs=-"
+
+// Check -E with -o.
+
+// RUN: %clang -### -E -o test.cui -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=OUT %s
+
+// RUN: %clang -### -E -o test.cui -save-temps -target x86_64-linux-gnu \
+// RUN: --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck -check-prefixes=OUT %s
+
+// OUT: {{.*}}clang-offload-bundler{{.*}}"-outputs=test.cui"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4604,6 +4604,17 @@
return Args.MakeArgString(Filename.c_str());
}
+static bool HasPreprocessOutput(const Action ) {
+ if (isa(JA))
+return true;
+ if (isa(JA) && isa(JA.getInputs()[0]))
+return true;
+ if (isa(JA) &&
+ HasPreprocessOutput(*(JA.getInputs()[0])))
+return true;
+ return false;
+}
+
const char *Driver::GetNamedOutputPath(Compilation , const JobAction ,
const char *BaseInput,
StringRef BoundArch, bool AtTopLevel,
@@ -4629,8 +4640,9 @@
}
// Default to writing to stdout?
- if (AtTopLevel && !CCGenDiagnostics && isa(JA))
+ if (AtTopLevel && !CCGenDiagnostics && HasPreprocessOutput(JA)) {
return "-";
+ }
// Is this the assembly listing for /FA?
if (JA.getType() ==
