[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

2019-07-18 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits 
gtbercea marked an inline comment as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2610
+  auto *GV = cast(Ptr);
+  GV->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
+

gtbercea wrote:
> ABataev wrote:
> > Better to fix the link clause processing in a different patch, it has 
> > nothing to do with the unified memory.
> Sure I can split this patch.
It turns out that the patch cannot be split. Splitting the patch would lead to 
an intermediate state which produced incorrect results for certain corner cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592



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

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

2019-07-17 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits 
gtbercea marked an inline comment as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2610
+  auto *GV = cast(Ptr);
+  GV->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
+

ABataev wrote:
> Better to fix the link clause processing in a different patch, it has nothing 
> to do with the unified memory.
Sure I can split this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592



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

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

2019-07-17 Thread Alexey Bataev via Phabricator via cfe-commits 
ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2610
+  auto *GV = cast(Ptr);
+  GV->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
+

Better to fix the link clause processing in a different patch, it has nothing 
to do with the unified memory.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592



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

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

2019-07-17 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits 
gtbercea updated this revision to Diff 210386.
gtbercea added a comment.

- Fix tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/declare_target_codegen.cpp
  test/OpenMP/declare_target_link_codegen.cpp
  test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp

Index: test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
===
--- test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
+++ test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
@@ -31,19 +31,19 @@
 }
 
 // CHECK-HOST: [[VAR:@.+]] = global double 1.00e+01
-// CHECK-HOST: [[VAR_DECL_TGT_LINK_PTR:@.+]] = global double* [[VAR]]
+// CHECK-HOST: [[VAR_DECL_TGT_LINK_PTR:@.+]] = weak global double* [[VAR]]
 
 // CHECK-HOST: [[TO_VAR:@.+]] = global double 2.00e+01
-// CHECK-HOST: [[VAR_DECL_TGT_TO_PTR:@.+]] = global double* [[TO_VAR]]
+// CHECK-HOST: [[VAR_DECL_TGT_TO_PTR:@.+]] = weak global double* [[TO_VAR]]
 
 // CHECK-HOST: [[OFFLOAD_SIZES:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 8]
 // CHECK-HOST: [[OFFLOAD_MAPTYPES:@.+]] = private unnamed_addr constant [2 x i64] [i64 800, i64 800]
 
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [21 x i8]
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* getelementptr inbounds ([21 x i8], [21 x i8]* [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries"
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [23 x i8]
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries"
 
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [24 x i8]
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*), i8* getelementptr inbounds ([24 x i8], [24 x i8]* [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 0, i32 0 }, section ".omp_offloading.entries"
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [26 x i8]
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*), i8* getelementptr inbounds ([26 x i8], [26 x i8]* [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 0, i32 0 }, section ".omp_offloading.entries"
 
 // CHECK-HOST: @llvm.used = appending global [2 x i8*] [i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*)], section "llvm.metadata"
 
@@ -75,8 +75,8 @@
 
 // CHECK-HOST: call i32 @__tgt_target(i64 -1, i8* @{{.*}}.region_id, i32 2, i8** [[BPTR7]], i8** [[BPTR8]], i64* getelementptr inbounds ([2 x i64], [2 x i64]* [[OFFLOAD_SIZES]], i32 0, i32 0), i64* getelementptr inbounds ([2 x i64], [2 x i64]* [[OFFLOAD_MAPTYPES]], i32 0, i32 0))
 
-// CHECK-DEVICE: [[VAR_LINK:@.+]] = common global double* null
-// CHECK-DEVICE: [[VAR_TO:@.+]] = common global double* null
+// CHECK-DEVICE: [[VAR_LINK:@.+]] = weak global double* null
+// CHECK-DEVICE: [[VAR_TO:@.+]] = weak global double* null
 
 // CHECK-DEVICE: @llvm.used = appending global [2 x i8*] [i8* bitcast (double** [[VAR_LINK]] to i8*), i8* bitcast (double** [[VAR_TO]] to i8*)], section "llvm.metadata"
 
Index: test/OpenMP/declare_target_link_codegen.cpp
===
--- test/OpenMP/declare_target_link_codegen.cpp
+++ test/OpenMP/declare_target_link_codegen.cpp
@@ -18,15 +18,15 @@
 #define HEADER
 
 // HOST-DAG: @c = external global i32,
-// HOST-DAG: @c_decl_tgt_ref_ptr = global i32* @c
+// HOST-DAG: @c_0_decl_tgt_ref_ptr = weak global i32* @c
 // DEVICE-NOT: @c =
-// DEVICE: @c_decl_tgt_ref_ptr = common global i32* null
+// DEVICE: @c_0_decl_tgt_ref_ptr = weak global i32* null
 // HOST: [[SIZES:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 4]
 // HOST: [[MAPTYPES:@.+]] = private unnamed_addr constant [2 x i64] [i64 35, i64 531]
-// HOST: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_decl_tgt_ref_ptr\00"
-// HOST: @.omp_offloading.entry.c_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { i8* bitcast (i32** @c_decl_tgt_ref_ptr to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name, i32 0, i32 0), i64

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

2019-07-11 Thread Alexey Bataev via Phabricator via cfe-commits 
ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2567
+  std::hash HashFn;
+  size_t hash = 0;
+  if (!CGM.getCodeGenOpts().MainFileName.empty())

1. Capitalize the first letter in tbe variable name.
2. Why do you need to use hash? Could you use variable sourcelocation instead?
Also, what if the variable is defined in one file but declared in another one 
file. Will it be linked correctly?



Comment at: lib/Sema/SemaOpenMP.cpp:2625
+  bool IsUsingUnifiedMemory =
+  Stack->hasRequiresDeclWithClause();
   if (VD->hasGlobalStorage() && CS && !CS->capturesVariable(VD) &&

I assume the changes in this file are from the different fix.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592



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

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

2019-07-11 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

Description lacking.
Why is that bad?
How does this play wrt reproducibility of the output?
Normally value names in IR are completely discarded in Release build mode, why 
do they matter here?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592



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

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

2019-07-11 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits 
gtbercea created this revision.
gtbercea added reviewers: ABataev, jdoerfert, caomhin.
Herald added subscribers: cfe-commits, guansong.
Herald added a project: clang.

This patch fixes the case where variables in different compilation units have 
the same name.


Repository:
  rC Clang

https://reviews.llvm.org/D64592

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/declare_target_codegen.cpp
  test/OpenMP/declare_target_link_codegen.cpp
  test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp

Index: test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
===
--- test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
+++ test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
@@ -39,11 +39,11 @@
 // CHECK-HOST: [[OFFLOAD_SIZES:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 8]
 // CHECK-HOST: [[OFFLOAD_MAPTYPES:@.+]] = private unnamed_addr constant [2 x i64] [i64 800, i64 800]
 
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [21 x i8]
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* getelementptr inbounds ([21 x i8], [21 x i8]* [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries"
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [23 x i8]
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries"
 
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [24 x i8]
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*), i8* getelementptr inbounds ([24 x i8], [24 x i8]* [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 0, i32 0 }, section ".omp_offloading.entries"
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [26 x i8]
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*), i8* getelementptr inbounds ([26 x i8], [26 x i8]* [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 0, i32 0 }, section ".omp_offloading.entries"
 
 // CHECK-HOST: @llvm.used = appending global [2 x i8*] [i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*)], section "llvm.metadata"
 
Index: test/OpenMP/declare_target_link_codegen.cpp
===
--- test/OpenMP/declare_target_link_codegen.cpp
+++ test/OpenMP/declare_target_link_codegen.cpp
@@ -18,15 +18,15 @@
 #define HEADER
 
 // HOST-DAG: @c = external global i32,
-// HOST-DAG: @c_decl_tgt_ref_ptr = global i32* @c
+// HOST-DAG: @c_0_decl_tgt_ref_ptr = global i32* @c
 // DEVICE-NOT: @c =
-// DEVICE: @c_decl_tgt_ref_ptr = common global i32* null
+// DEVICE: @c_0_decl_tgt_ref_ptr = common global i32* null
 // HOST: [[SIZES:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 4]
 // HOST: [[MAPTYPES:@.+]] = private unnamed_addr constant [2 x i64] [i64 35, i64 531]
-// HOST: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_decl_tgt_ref_ptr\00"
-// HOST: @.omp_offloading.entry.c_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { i8* bitcast (i32** @c_decl_tgt_ref_ptr to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name, i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries", align 1
-// DEVICE-NOT: internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_decl_tgt_ref_ptr\00"
-// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32** @c_decl_tgt_ref_ptr to i8*)]
+// HOST: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_0_decl_tgt_ref_ptr\00"
+// HOST: @.omp_offloading.entry.c_0_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { i8* bitcast (i32** @c_0_decl_tgt_ref_ptr to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name, i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries", align 1
+// DEVICE-NOT: internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_0_decl_tgt_ref_ptr\00"
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32** @c_0_decl_tgt_ref_ptr to i8*)]
 
 extern int c;
 #pragma omp declare target link(c)
@@ -44,7 +44,7 @@
 }
 
 // DEVICE: define weak void @__omp_offloadin