[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl reclaimed this revision.
yaxunl added a comment.

we still need this


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

https://reviews.llvm.org/D57829



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl abandoned this revision.
yaxunl marked an inline comment as done.
yaxunl added a comment.

no longer needed


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

https://reviews.llvm.org/D57829



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Also, why is this HIP-specific?  I thought the toolchain was largely shared 
with CUDA and there were just a few runtime differences.


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

https://reviews.llvm.org/D57829



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D57829#1387416 , @yaxunl wrote:

> In D57829#1387412 , @tra wrote:
>
> > Could you elaborate on why you want to disable this metadata? I think the 
> > original idea of llvm.linker.options was that it should be ignored if the 
> > back-end does not support it.
>
>
> If backend does not support it, it goes to 
> TargetLoweringObjectFileELF::emitModuleMetadata and causes codegen to fail.


Fails how? AFAICT, for llvm.linker.options  emitModuleMetadata() just creates 
an ELF section ".linker-options" which should just work.
Are you saying that some of HIP tools can't deal with the unknown section type?


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

https://reviews.llvm.org/D57829



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D57829#1387412 , @tra wrote:

> Could you elaborate on why you want to disable this metadata? I think the 
> original idea of llvm.linker.options was that it should be ignored if the 
> back-end does not support it.


If backend does not support it, it goes to 
TargetLoweringObjectFileELF::emitModuleMetadata and causes codegen to fail.


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

https://reviews.llvm.org/D57829



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Could you elaborate on why you want to disable this metadata? I think the 
original idea of llvm.linker.options was that it should be ignored if the 
back-end does not support it.




Comment at: lib/CodeGen/CodeGenModule.cpp:441
   if (CodeGenOpts.Autolink &&
+  !(Context.getLangOpts().CUDAIsDevice && Context.getLangOpts().HIP) &&
   (Context.getLangOpts().Modules || !LinkerOptionsMetadata.empty())) {

If we do need to disable it, it may be better to do it in 
`ShouldDisableAutolink()` function in clang/lib/Driver/ToolChains/Clang.cpp




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

https://reviews.llvm.org/D57829



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.

HIP toolchain does not support llvm.linker.options in device compilation, 
therefore disable it.


https://reviews.llvm.org/D57829

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCUDA/linker-options.cu


Index: test/CodeGenCUDA/linker-options.cu
===
--- /dev/null
+++ test/CodeGenCUDA/linker-options.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -x hip %s | FileCheck %s
+
+// CHECK-NOT: llvm.linker.options
+#pragma comment(lib, "a.so")
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -438,6 +438,7 @@
 SanStats->finish();
 
   if (CodeGenOpts.Autolink &&
+  !(Context.getLangOpts().CUDAIsDevice && Context.getLangOpts().HIP) &&
   (Context.getLangOpts().Modules || !LinkerOptionsMetadata.empty())) {
 EmitModuleLinkOptions();
   }


Index: test/CodeGenCUDA/linker-options.cu
===
--- /dev/null
+++ test/CodeGenCUDA/linker-options.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -x hip %s | FileCheck %s
+
+// CHECK-NOT: llvm.linker.options
+#pragma comment(lib, "a.so")
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -438,6 +438,7 @@
 SanStats->finish();
 
   if (CodeGenOpts.Autolink &&
+  !(Context.getLangOpts().CUDAIsDevice && Context.getLangOpts().HIP) &&
   (Context.getLangOpts().Modules || !LinkerOptionsMetadata.empty())) {
 EmitModuleLinkOptions();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits