jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, ABataev.
Herald added subscribers: asavonic, guansong, yaxunl.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This patch removes the special-case handling of visibility when
compiling for an OpenMP target offloading device. This was orignally
added as a precaution against the bug encountered in PR41826 when
symbols in the device were being preempted by shared library symbols.
This should instead by done to more specifically disable symbol
preemption on the device and allow the user to control device visibility
more directly. This is done by passing the `-Bsymbolic` flag to the
toolchain, indicating that all symbols bind locally and cannot bind to
another symbol at runtime. It is assumed now that when we compile for the
device, no symbol addresses should be preempted by pending shared library
loads.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117806

Files:
  clang/lib/AST/Decl.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
  clang/test/OpenMP/nvptx_target_pure_deleted_codegen.cpp
  clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
  clang/test/OpenMP/target_attribute_convergent.cpp

Index: clang/test/OpenMP/target_attribute_convergent.cpp
===================================================================
--- clang/test/OpenMP/target_attribute_convergent.cpp
+++ clang/test/OpenMP/target_attribute_convergent.cpp
@@ -9,5 +9,5 @@
 #pragma omp end declare target
 
 // CHECK: Function Attrs: {{.*}}convergent{{.*}}
-// CHECK: define hidden void @_Z3foov() [[ATTRIBUTE_NUMBER:#[0-9]+]]
+// CHECK: define dso_local void @_Z3foov() [[ATTRIBUTE_NUMBER:#[0-9]+]]
 // CHECK: attributes [[ATTRIBUTE_NUMBER]] = { {{.*}}convergent{{.*}} }
Index: clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
===================================================================
--- clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
@@ -34,18 +34,18 @@
 #pragma omp declare target
 T a = T();
 T f = a;
-// CHECK: define{{ hidden | }}void @{{.+}}foo{{.+}}([[T]]* noundef byval([[T]]) align {{.+}})
+// CHECK: define{{ dso_local | }}void @{{.+}}foo{{.+}}([[T]]* noundef byval([[T]]) align {{.+}})
 void foo(T a = T()) {
   return;
 }
-// CHECK: define{{ hidden | }}[6 x i64] @{{.+}}bar{{.+}}()
+// CHECK: define{{ dso_local | }}[6 x i64] @{{.+}}bar{{.+}}()
 T bar() {
 // CHECK:      bitcast [[T]]* %{{.+}} to [6 x i64]*
 // CHECK-NEXT: load [6 x i64], [6 x i64]* %{{.+}},
 // CHECK-NEXT: ret [6 x i64]
   return T();
 }
-// CHECK: define{{ hidden | }}void @{{.+}}baz{{.+}}()
+// CHECK: define{{ dso_local | }}void @{{.+}}baz{{.+}}()
 void baz() {
 // CHECK:      call [6 x i64] @{{.+}}bar{{.+}}()
 // CHECK-NEXT: bitcast [[T]]* %{{.+}} to [6 x i64]*
@@ -54,17 +54,17 @@
 }
 T1 a1 = T1();
 T1 f1 = a1;
-// CHECK: define{{ hidden | }}void @{{.+}}foo1{{.+}}([[T1]]* noundef byval([[T1]]) align {{.+}})
+// CHECK: define{{ dso_local | }}void @{{.+}}foo1{{.+}}([[T1]]* noundef byval([[T1]]) align {{.+}})
 void foo1(T1 a = T1()) {
   return;
 }
-// CHECK: define{{ hidden | }}[[T1]] @{{.+}}bar1{{.+}}()
+// CHECK: define{{ dso_local | }}[[T1]] @{{.+}}bar1{{.+}}()
 T1 bar1() {
 // CHECK:      load [[T1]], [[T1]]*
 // CHECK-NEXT: ret [[T1]]
   return T1();
 }
-// CHECK: define{{ hidden | }}void @{{.+}}baz1{{.+}}()
+// CHECK: define{{ dso_local | }}void @{{.+}}baz1{{.+}}()
 void baz1() {
 // CHECK: call [[T1]] @{{.+}}bar1{{.+}}()
   T1 t = bar1();
Index: clang/test/OpenMP/nvptx_target_pure_deleted_codegen.cpp
===================================================================
--- clang/test/OpenMP/nvptx_target_pure_deleted_codegen.cpp
+++ clang/test/OpenMP/nvptx_target_pure_deleted_codegen.cpp
@@ -10,8 +10,8 @@
 #define HEADER
 
 // CHECK-NOT: class_type_info
-// CHECK-DAG: @_ZTV7Derived = linkonce_odr hidden unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%class.Derived*)* @_ZN7Derived3fooEv to i8*)] }
-// CHECK-DAG: @_ZTV4Base = linkonce_odr hidden unnamed_addr constant { [3 x i8*] } zeroinitializer
+// CHECK-DAG: @_ZTV7Derived = linkonce_odr unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%class.Derived*)* @_ZN7Derived3fooEv to i8*)] }
+// CHECK-DAG: @_ZTV4Base = linkonce_odr unnamed_addr constant { [3 x i8*] } zeroinitializer
 // CHECK-NOT: class_type_info
 class Base {
   public:
Index: clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
===================================================================
--- clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
+++ clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
@@ -16,7 +16,7 @@
 // SIMD-ONLY-NOT: {{__kmpc|__tgt}}
 
 // DEVICE-DAG: [[C_ADDR:.+]] = internal global i32 0,
-// DEVICE-DAG: [[CD_ADDR:@.+]] ={{ hidden | }}global %struct.S zeroinitializer,
+// DEVICE-DAG: [[CD_ADDR:@.+]] = global %struct.S zeroinitializer,
 // HOST-DAG: @[[C_ADDR:.+]] = internal global i32 0,
 // HOST-DAG: @[[CD_ADDR:.+]] ={{( hidden | dso_local)?}} global %struct.S zeroinitializer,
 
@@ -34,12 +34,12 @@
 #pragma omp declare target (bar)
 int caz() { return 0; }
 
-// DEVICE-DAG: define{{ hidden | }}noundef i32 [[FOO:@.*foo.*]]()
-// DEVICE-DAG: define{{ hidden | }}noundef i32 [[BAR:@.*bar.*]]()
-// DEVICE-DAG: define{{ hidden | }}noundef i32 [[BAZ:@.*baz.*]]()
-// DEVICE-DAG: define{{ hidden | }}noundef i32 [[DOO:@.*doo.*]]()
-// DEVICE-DAG: define{{ hidden | }}noundef i32 [[CAR:@.*car.*]]()
-// DEVICE-DAG: define{{ hidden | }}noundef i32 [[CAZ:@.*caz.*]]()
+// DEVICE-DAG: define{{ dso_local | }}noundef i32 [[FOO:@.*foo.*]]()
+// DEVICE-DAG: define{{ dso_local | }}noundef i32 [[BAR:@.*bar.*]]()
+// DEVICE-DAG: define{{ dso_local | }}noundef i32 [[BAZ:@.*baz.*]]()
+// DEVICE-DAG: define{{ dso_local | }}noundef i32 [[DOO:@.*doo.*]]()
+// DEVICE-DAG: define{{ dso_local | }}noundef i32 [[CAR:@.*car.*]]()
+// DEVICE-DAG: define{{ dso_local | }}noundef i32 [[CAZ:@.*caz.*]]()
 
 static int c = foo() + bar() + baz();
 #pragma omp declare target (c)
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -406,7 +406,7 @@
       (ToolChain.getTriple().getVendor() != llvm::Triple::MipsTechnologies);
 
   ArgStringList CmdArgs;
-
+  
   // Silence warning for "clang -g foo.o -o foo"
   Args.ClaimAllArgs(options::OPT_g_Group);
   // and "clang -emit-llvm foo.o -o foo"
@@ -577,6 +577,11 @@
     }
     CmdArgs.push_back("-lm");
   }
+
+  // If we are linking for the device all symbols should be bound locally.
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP))
+    CmdArgs.push_back("-Bsymbolic");
+
   // Silence warnings when linking C code with a C++ '-stdlib' argument.
   Args.ClaimAllArgs(options::OPT_stdlib_EQ);
 
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -787,10 +787,6 @@
     // Note that we don't want to make the variable non-external
     // because of this, but unique-external linkage suits us.
 
-    // We need variables inside OpenMP declare target directives to be visible.
-    if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Var))
-      return LinkageInfo::external();
-
     if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var) &&
         !IgnoreVarTypeLinkage) {
       LinkageInfo TypeLV = getLVForType(*Var->getType(), computation);
@@ -917,10 +913,6 @@
   if (!isExternallyVisible(LV.getLinkage()))
     return LinkageInfo(LV.getLinkage(), DefaultVisibility, false);
 
-  // Mark the symbols as hidden when compiling for the device.
-  if (Context.getLangOpts().OpenMP && Context.getLangOpts().OpenMPIsDevice)
-    LV.mergeVisibility(HiddenVisibility, /*newExplicit=*/false);
-
   return LV;
 }
 
@@ -1075,11 +1067,6 @@
   // Finally, merge in information from the class.
   LV.mergeMaybeWithVisibility(classLV, considerClassVisibility);
 
-  // We need variables inside OpenMP declare target directives to be visible.
-  if (const VarDecl *VD = dyn_cast<VarDecl>(D))
-    if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD))
-      return LinkageInfo(LV.getLinkage(), DefaultVisibility, false);
-
   return LV;
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D117806: [OpenMP] Rem... Joseph Huber via Phabricator via cfe-commits

Reply via email to