[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-15 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe8090d83fd92: [HIP] Do not call opt/llc for -fno-gpu-rdc 
(authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81627

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/cuda-phases.cu
  clang/test/Driver/hip-binding.hip
  clang/test/Driver/hip-device-compile.hip
  clang/test/Driver/hip-phases.hip
  clang/test/Driver/hip-save-temps.hip
  clang/test/Driver/hip-toolchain-mllvm.hip
  clang/test/Driver/hip-toolchain-no-rdc.hip
  clang/test/Driver/hip-toolchain-opt.hip

Index: clang/test/Driver/hip-toolchain-opt.hip
===
--- clang/test/Driver/hip-toolchain-opt.hip
+++ clang/test/Driver/hip-toolchain-opt.hip
@@ -68,27 +68,9 @@
 // Oz-SAME: "-Oz"
 // Og-SAME: "-Og"
 
-// ALL: "{{.*}}opt"
-// DEFAULT-NOT: "-O{{.}}"
-// O0-SAME: "-O0"
-// O1-SAME: "-O1"
-// O2-SAME: "-O2"
-// O3-SAME: "-O3"
-// Os-SAME: "-Os"
-// Oz-SAME: "-Oz"
-// Og-SAME: "-O1"
-// ALL-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// ALL-NOT: "{{.*}}opt"
 
-// ALL: "{{.*}}llc"
-// DEFAULT-NOT: "-O{{.}}"
-// O0-SAME: "-O0"
-// O1-SAME: "-O1"
-// O2-SAME: "-O2"
-// O3-SAME: "-O3"
-// Os-SAME: "-O2"
-// Oz-SAME: "-O2"
-// Og-SAME: "-O1"
-// ALL-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// ALL-NOT: "{{.*}}llc"
 
 // ALL: "{{.*}}clang{{.*}}" "-cc1" "-triple" "x86_64-unknown-linux-gnu"
 // DEFAULT-NOT: "-O{{.}}"
Index: clang/test/Driver/hip-toolchain-no-rdc.hip
===
--- clang/test/Driver/hip-toolchain-no-rdc.hip
+++ clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -18,25 +18,17 @@
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
-// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: "-emit-obj"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fcuda-allow-variadic-functions" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
 // CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
-// CHECK-SAME: {{.*}} "-o" [[A_BC_803:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_803:".*o"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
-// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_803]]
-// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_803:".*-gfx803-linked-.*bc"]]
-
-// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx803"
-// CHECK-SAME: "-o" [[OPT_BC_DEV_A_803:".*-gfx803-optimized.*bc"]]
-
-// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx803"
-// CHECK-SAME: "-filetype=obj"
-// CHECK-SAME: "-o" [[OBJ_DEV_A_803:".*-gfx803-.*o"]]
+// CHECK-NOT: {{".*llvm-link"}}
+// CHECK-NOT: {{".*opt"}}
+// CHECK-NOT: {{".*llc"}}
 
 // CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
@@ -47,25 +39,17 @@
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
-// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: "-emit-obj"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" "-fcuda-allow-variadic-functions" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
 // CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
-// CHECK-SAME: {{.*}} "-o" [[A_BC_900:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_900:".*o"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
-// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_900]]
-// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_900:".*-gfx900-linked-.*bc"]]
-
-// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx900"
-// CHECK-SAME: "-o" [[OPT_BC_DEV_A_900:".*-gfx900-optimized.*bc"]]
-
-// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx900"
-// CHECK-SAME: "-filetype=obj"
-// CHECK-SAME: "-o" [[OBJ_DEV_A_900:".*-gfx900-.*o"]]
+// CHECK-NOT: {{".*llvm-link"}}
+// CHECK-NOT: {{".*opt"}}
+// CHECK-NOT: {{".*llc"}}
 
 // CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
@@ -92,25 +76,17 @@
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
-// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: "-emit-obj"
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fcuda-allow-variadic-functions" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
 // CHECK-SAME: 

[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-15 Thread Aaron Enye Shi via Phabricator via cfe-commits
ashi1 accepted this revision.
ashi1 added a comment.

LGTM, thank you for splitting the cuda-phases from hip-phases, and opening up a 
follow-up patch for RDC case.


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

https://reviews.llvm.org/D81627



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


[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-12 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D81627#2090499 , @tra wrote:

> LGTM.  Good to go if @arsenm is OK with fixing -fgpu-rdc in a separate patch.


@arsenm Are you OK with fixing -fgpu-rdc in a separate patch? The fix for that 
is orthogonal to the current patch. Mixing them together will clutter things 
up. Also the need for -fgpu-rdc is not so urgent since only rccl needs it. 
Requesting this patch to fix -fgpu-rdc will delay fixing issues for all other 
math libs and frameworks.


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

https://reviews.llvm.org/D81627



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


[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-12 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM.  Good to go if @arsenm is OK with fixing -fgpu-rdc in a separate patch.


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

https://reviews.llvm.org/D81627



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


[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-11 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 4 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:2725-2726
 for (unsigned I = 0, E = GpuArchList.size(); I != E; ++I) {
   // Create a link action to link device IR with device library
   // and generate ISA.
+  CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(

tra wrote:
> The comment about "create link action" should probably be moved down below to 
> where the link action is constructed now.
> 
will do



Comment at: clang/lib/Driver/Driver.cpp:2727-2732
+  CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(
+  C, Args, phases::Backend, CudaDeviceActions[I],
+  AssociatedOffloadKind);
+  CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(
+  C, Args, phases::Assemble, CudaDeviceActions[I],
+  AssociatedOffloadKind);

tra wrote:
> Looks like we're chaining backend/assembly actions here. but it's not easy to 
> spot that we use `CudaDeviceActions[I]`  as an intermediate value. At the 
> first glance it looked like a copy/paste error writing to 
> `CudaDeviceActions[I]` multiple times.
> 
> It would be easier to see what's going on if the code was structured like 
> this:
> ```
> BackendAction = Construct(... CudaDeviceActions[I]);
> AssembleAction  = Construct(... BackendAction);
> AL.push_back(AssembleAction)
> CudaDeviceActions[I] = C.MakeAction(AL);
> ```
> 
will do


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

https://reviews.llvm.org/D81627



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


[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-11 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 270302.
yaxunl marked 2 inline comments as done.
yaxunl added a reviewer: ashi1.
yaxunl added a comment.

revised by Artem's comments.


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

https://reviews.llvm.org/D81627

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/cuda-phases.cu
  clang/test/Driver/hip-binding.hip
  clang/test/Driver/hip-device-compile.hip
  clang/test/Driver/hip-phases.hip
  clang/test/Driver/hip-save-temps.hip
  clang/test/Driver/hip-toolchain-mllvm.hip
  clang/test/Driver/hip-toolchain-no-rdc.hip
  clang/test/Driver/hip-toolchain-opt.hip

Index: clang/test/Driver/hip-toolchain-opt.hip
===
--- clang/test/Driver/hip-toolchain-opt.hip
+++ clang/test/Driver/hip-toolchain-opt.hip
@@ -68,27 +68,9 @@
 // Oz-SAME: "-Oz"
 // Og-SAME: "-Og"
 
-// ALL: "{{.*}}opt"
-// DEFAULT-NOT: "-O{{.}}"
-// O0-SAME: "-O0"
-// O1-SAME: "-O1"
-// O2-SAME: "-O2"
-// O3-SAME: "-O3"
-// Os-SAME: "-Os"
-// Oz-SAME: "-Oz"
-// Og-SAME: "-O1"
-// ALL-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// ALL-NOT: "{{.*}}opt"
 
-// ALL: "{{.*}}llc"
-// DEFAULT-NOT: "-O{{.}}"
-// O0-SAME: "-O0"
-// O1-SAME: "-O1"
-// O2-SAME: "-O2"
-// O3-SAME: "-O3"
-// Os-SAME: "-O2"
-// Oz-SAME: "-O2"
-// Og-SAME: "-O1"
-// ALL-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// ALL-NOT: "{{.*}}llc"
 
 // ALL: "{{.*}}clang{{.*}}" "-cc1" "-triple" "x86_64-unknown-linux-gnu"
 // DEFAULT-NOT: "-O{{.}}"
Index: clang/test/Driver/hip-toolchain-no-rdc.hip
===
--- clang/test/Driver/hip-toolchain-no-rdc.hip
+++ clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -18,25 +18,17 @@
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
-// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: "-emit-obj"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fcuda-allow-variadic-functions" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
 // CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
-// CHECK-SAME: {{.*}} "-o" [[A_BC_803:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_803:".*o"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
-// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_803]]
-// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_803:".*-gfx803-linked-.*bc"]]
-
-// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx803"
-// CHECK-SAME: "-o" [[OPT_BC_DEV_A_803:".*-gfx803-optimized.*bc"]]
-
-// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx803"
-// CHECK-SAME: "-filetype=obj"
-// CHECK-SAME: "-o" [[OBJ_DEV_A_803:".*-gfx803-.*o"]]
+// CHECK-NOT: {{".*llvm-link"}}
+// CHECK-NOT: {{".*opt"}}
+// CHECK-NOT: {{".*llc"}}
 
 // CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
@@ -47,25 +39,17 @@
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
-// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: "-emit-obj"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" "-fcuda-allow-variadic-functions" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
 // CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
-// CHECK-SAME: {{.*}} "-o" [[A_BC_900:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_900:".*o"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
-// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_900]]
-// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_900:".*-gfx900-linked-.*bc"]]
-
-// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx900"
-// CHECK-SAME: "-o" [[OPT_BC_DEV_A_900:".*-gfx900-optimized.*bc"]]
-
-// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx900"
-// CHECK-SAME: "-filetype=obj"
-// CHECK-SAME: "-o" [[OBJ_DEV_A_900:".*-gfx900-.*o"]]
+// CHECK-NOT: {{".*llvm-link"}}
+// CHECK-NOT: {{".*opt"}}
+// CHECK-NOT: {{".*llc"}}
 
 // CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
@@ -92,25 +76,17 @@
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
-// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: "-emit-obj"
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fcuda-allow-variadic-functions" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
 // CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
-// CHECK-SAME: {{.*}} "-o" 

[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-11 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D81627#2088502 , @arsenm wrote:

> In D81627#2088500 , @arsenm wrote:
>
> > It doesn't matter if we don't support isa linking. We should just use clang 
> > and default to -flto. LTO "just works" as is
>
>
> This is a step forward, but the lack of ISA linking shouldn't block 
> eliminating the use of llc/opt


Agree. Will fix -fgpu-rdc in a different patch.


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

https://reviews.llvm.org/D81627



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


[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-11 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

In D81627#2088500 , @arsenm wrote:

> It doesn't matter if we don't support isa linking. We should just use clang 
> and default to -flto. LTO "just works" as is


This is a step forward, but the lack of ISA linking shouldn't block eliminating 
the use of llc/opt


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

https://reviews.llvm.org/D81627



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


[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-11 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

It doesn't matter if we don't support isa linking. We should just use clang and 
default to -flto. LTO "just works" as is


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

https://reviews.llvm.org/D81627



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


[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-11 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

Looks OK in general. I'm happy to see reduced opt/llc use.

You may want to get someone more familiar with the AMD GPU compilation process 
to double-check that the compilation pipeline still does the right thing.




Comment at: clang/lib/Driver/Driver.cpp:2725-2726
 for (unsigned I = 0, E = GpuArchList.size(); I != E; ++I) {
   // Create a link action to link device IR with device library
   // and generate ISA.
+  CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(

The comment about "create link action" should probably be moved down below to 
where the link action is constructed now.




Comment at: clang/lib/Driver/Driver.cpp:2727-2732
+  CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(
+  C, Args, phases::Backend, CudaDeviceActions[I],
+  AssociatedOffloadKind);
+  CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(
+  C, Args, phases::Assemble, CudaDeviceActions[I],
+  AssociatedOffloadKind);

Looks like we're chaining backend/assembly actions here. but it's not easy to 
spot that we use `CudaDeviceActions[I]`  as an intermediate value. At the first 
glance it looked like a copy/paste error writing to `CudaDeviceActions[I]` 
multiple times.

It would be easier to see what's going on if the code was structured like this:
```
BackendAction = Construct(... CudaDeviceActions[I]);
AssembleAction  = Construct(... BackendAction);
AL.push_back(AssembleAction)
CudaDeviceActions[I] = C.MakeAction(AL);
```



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

https://reviews.llvm.org/D81627



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


[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

2020-06-10 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a subscriber: tpr.

Currently HIP toolchain calls clang to emit bitcode then calls opt/llc for 
device compilation for the default -fno-gpu-rdc
case, which is unnecessary since clang is able to compile a single source file 
to ISA.

This patch fixes the HIP action builder and toolchain so that the default 
-fno-gpu-rdc can be done like a canonical
toolchain, i.e. one clang -cc1 invocation to compile source code to ISA.

This can avoid unnecessary processes to speed up the compilation, and avoid 
redundant LLVM passes which are
performed in clang -cc1 and opt.

This patch does not remove opt/llc in -fgpu-rdc case since device linking is 
still needed whereas
amdgpu backend does not support ISA linking for now.


https://reviews.llvm.org/D81627

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/cuda-phases.cu
  clang/test/Driver/hip-binding.hip
  clang/test/Driver/hip-device-compile.hip
  clang/test/Driver/hip-phases.hip
  clang/test/Driver/hip-save-temps.hip
  clang/test/Driver/hip-toolchain-mllvm.hip
  clang/test/Driver/hip-toolchain-no-rdc.hip
  clang/test/Driver/hip-toolchain-opt.hip

Index: clang/test/Driver/hip-toolchain-opt.hip
===
--- clang/test/Driver/hip-toolchain-opt.hip
+++ clang/test/Driver/hip-toolchain-opt.hip
@@ -68,27 +68,9 @@
 // Oz-SAME: "-Oz"
 // Og-SAME: "-Og"
 
-// ALL: "{{.*}}opt"
-// DEFAULT-NOT: "-O{{.}}"
-// O0-SAME: "-O0"
-// O1-SAME: "-O1"
-// O2-SAME: "-O2"
-// O3-SAME: "-O3"
-// Os-SAME: "-Os"
-// Oz-SAME: "-Oz"
-// Og-SAME: "-O1"
-// ALL-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// ALL-NOT: "{{.*}}opt"
 
-// ALL: "{{.*}}llc"
-// DEFAULT-NOT: "-O{{.}}"
-// O0-SAME: "-O0"
-// O1-SAME: "-O1"
-// O2-SAME: "-O2"
-// O3-SAME: "-O3"
-// Os-SAME: "-O2"
-// Oz-SAME: "-O2"
-// Og-SAME: "-O1"
-// ALL-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// ALL-NOT: "{{.*}}llc"
 
 // ALL: "{{.*}}clang{{.*}}" "-cc1" "-triple" "x86_64-unknown-linux-gnu"
 // DEFAULT-NOT: "-O{{.}}"
Index: clang/test/Driver/hip-toolchain-no-rdc.hip
===
--- clang/test/Driver/hip-toolchain-no-rdc.hip
+++ clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -18,25 +18,17 @@
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
-// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: "-emit-obj"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fcuda-allow-variadic-functions" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
 // CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
-// CHECK-SAME: {{.*}} "-o" [[A_BC_803:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_803:".*o"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
-// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_803]]
-// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_803:".*-gfx803-linked-.*bc"]]
-
-// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx803"
-// CHECK-SAME: "-o" [[OPT_BC_DEV_A_803:".*-gfx803-optimized.*bc"]]
-
-// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx803"
-// CHECK-SAME: "-filetype=obj"
-// CHECK-SAME: "-o" [[OBJ_DEV_A_803:".*-gfx803-.*o"]]
+// CHECK-NOT: {{".*llvm-link"}}
+// CHECK-NOT: {{".*opt"}}
+// CHECK-NOT: {{".*llc"}}
 
 // CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
@@ -47,25 +39,17 @@
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
-// CHECK-SAME: "-emit-llvm-bc"
+// CHECK-SAME: "-emit-obj"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" "-fcuda-allow-variadic-functions" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
 // CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
-// CHECK-SAME: {{.*}} "-o" [[A_BC_900:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_900:".*o"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
-// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_900]]
-// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_900:".*-gfx900-linked-.*bc"]]
-
-// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx900"
-// CHECK-SAME: "-o" [[OPT_BC_DEV_A_900:".*-gfx900-optimized.*bc"]]
-
-// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx900"
-// CHECK-SAME: "-filetype=obj"
-// CHECK-SAME: "-o" [[OBJ_DEV_A_900:".*-gfx900-.*o"]]
+// CHECK-NOT: {{".*llvm-link"}}
+// CHECK-NOT: {{".*opt"}}
+// CHECK-NOT: {{".*llc"}}
 
 // CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o"