[PATCH] D74925: [OpenMP][Opt] Combine `struct ident_t*` during deduplication

2020-02-25 Thread Johannes Doerfert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG396b7253944e: [OpenMP][Opt] Combine `struct ident_t*` during 
deduplication (authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74925

Files:
  clang/test/OpenMP/PR44893.c
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/deduplication.ll
  llvm/test/Transforms/OpenMP/gtid.ll

Index: llvm/test/Transforms/OpenMP/gtid.ll
===
--- llvm/test/Transforms/OpenMP/gtid.ll
+++ /dev/null
@@ -1,86 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
-; RUN: opt -openmpopt -S < %s | FileCheck %s
-; RUN: opt -passes=openmpopt -S < %s | FileCheck %s
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-
-%struct.ident_t = type { i32, i32, i32, i32, i8* }
-
-@0 = private unnamed_addr global %struct.ident_t { i32 0, i32 34, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str, i32 0, i32 0) }, align 8
-@.str = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
-
-declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
-declare void @useI32(i32)
-
-define void @external(i1 %c) {
-; CHECK-LABEL: define {{[^@]+}}@external
-; CHECK-SAME: (i1 [[C:%.*]])
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:[[C2:%.*]] = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-; CHECK-NEXT:br i1 [[C]], label [[T:%.*]], label [[E:%.*]]
-; CHECK:   t:
-; CHECK-NEXT:call void @internal(i32 [[C2]], i32 [[C2]])
-; CHECK-NEXT:call void @useI32(i32 [[C2]])
-; CHECK-NEXT:br label [[M:%.*]]
-; CHECK:   e:
-; CHECK-NEXT:call void @internal(i32 [[C2]], i32 [[C2]])
-; CHECK-NEXT:call void @useI32(i32 [[C2]])
-; CHECK-NEXT:br label [[M]]
-; CHECK:   m:
-; CHECK-NEXT:call void @internal(i32 0, i32 [[C2]])
-; CHECK-NEXT:call void @useI32(i32 [[C2]])
-; CHECK-NEXT:ret void
-;
-entry:
-  br i1 %c, label %t, label %e
-t:
-  %c0 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @internal(i32 %c0, i32 %c0)
-  call void @useI32(i32 %c0)
-  br label %m
-e:
-  %c1 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @internal(i32 %c1, i32 %c1)
-  call void @useI32(i32 %c1)
-  br label %m
-m:
-  %c2 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @internal(i32 0, i32 %c2)
-  call void @useI32(i32 %c2)
-  ret void
-}
-
-define internal void @internal(i32 %not_gtid, i32 %gtid) {
-; CHECK-LABEL: define {{[^@]+}}@internal
-; CHECK-SAME: (i32 [[NOT_GTID:%.*]], i32 [[GTID:%.*]])
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:[[C:%.*]] = icmp eq i32 [[GTID]], [[GTID]]
-; CHECK-NEXT:br i1 [[C]], label [[T:%.*]], label [[E:%.*]]
-; CHECK:   t:
-; CHECK-NEXT:call void @useI32(i32 [[GTID]])
-; CHECK-NEXT:call void @external(i1 [[C]])
-; CHECK-NEXT:br label [[M:%.*]]
-; CHECK:   e:
-; CHECK-NEXT:call void @useI32(i32 [[GTID]])
-; CHECK-NEXT:br label [[M]]
-; CHECK:   m:
-; CHECK-NEXT:call void @useI32(i32 [[GTID]])
-; CHECK-NEXT:ret void
-;
-entry:
-  %cc = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  %c = icmp eq i32 %cc, %gtid
-  br i1 %c, label %t, label %e
-t:
-  %c0 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @useI32(i32 %c0)
-  call void @external(i1 %c)
-  br label %m
-e:
-  %c1 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @useI32(i32 %c1)
-  br label %m
-m:
-  %c2 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @useI32(i32 %c2)
-  ret void
-}
Index: llvm/test/Transforms/OpenMP/deduplication.ll
===
--- /dev/null
+++ llvm/test/Transforms/OpenMP/deduplication.ll
@@ -0,0 +1,223 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
+; RUN: opt -openmpopt -S < %s | FileCheck %s
+; RUN: opt -passes=openmpopt -S < %s | FileCheck %s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+%struct.ident_t = type { i32, i32, i32, i32, i8* }
+
+@0 = private unnamed_addr global %struct.ident_t { i32 0, i32 34, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str0, i32 0, i32 0) }, align 8
+@1 = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str1, i32 0, i32 0) }, align 8
+@2 = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* 

[PATCH] D74925: [OpenMP][Opt] Combine `struct ident_t*` during deduplication

2020-02-24 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

Nice. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74925



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


[PATCH] D74925: [OpenMP][Opt] Combine `struct ident_t*` during deduplication

2020-02-24 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert updated this revision to Diff 246256.
jdoerfert marked an inline comment as done.
jdoerfert added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add tests, addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74925

Files:
  clang/test/OpenMP/PR44893.c
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/deduplication.ll
  llvm/test/Transforms/OpenMP/gtid.ll

Index: llvm/test/Transforms/OpenMP/gtid.ll
===
--- llvm/test/Transforms/OpenMP/gtid.ll
+++ /dev/null
@@ -1,86 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
-; RUN: opt -openmpopt -S < %s | FileCheck %s
-; RUN: opt -passes=openmpopt -S < %s | FileCheck %s
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-
-%struct.ident_t = type { i32, i32, i32, i32, i8* }
-
-@0 = private unnamed_addr global %struct.ident_t { i32 0, i32 34, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str, i32 0, i32 0) }, align 8
-@.str = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
-
-declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
-declare void @useI32(i32)
-
-define void @external(i1 %c) {
-; CHECK-LABEL: define {{[^@]+}}@external
-; CHECK-SAME: (i1 [[C:%.*]])
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:[[C2:%.*]] = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-; CHECK-NEXT:br i1 [[C]], label [[T:%.*]], label [[E:%.*]]
-; CHECK:   t:
-; CHECK-NEXT:call void @internal(i32 [[C2]], i32 [[C2]])
-; CHECK-NEXT:call void @useI32(i32 [[C2]])
-; CHECK-NEXT:br label [[M:%.*]]
-; CHECK:   e:
-; CHECK-NEXT:call void @internal(i32 [[C2]], i32 [[C2]])
-; CHECK-NEXT:call void @useI32(i32 [[C2]])
-; CHECK-NEXT:br label [[M]]
-; CHECK:   m:
-; CHECK-NEXT:call void @internal(i32 0, i32 [[C2]])
-; CHECK-NEXT:call void @useI32(i32 [[C2]])
-; CHECK-NEXT:ret void
-;
-entry:
-  br i1 %c, label %t, label %e
-t:
-  %c0 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @internal(i32 %c0, i32 %c0)
-  call void @useI32(i32 %c0)
-  br label %m
-e:
-  %c1 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @internal(i32 %c1, i32 %c1)
-  call void @useI32(i32 %c1)
-  br label %m
-m:
-  %c2 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @internal(i32 0, i32 %c2)
-  call void @useI32(i32 %c2)
-  ret void
-}
-
-define internal void @internal(i32 %not_gtid, i32 %gtid) {
-; CHECK-LABEL: define {{[^@]+}}@internal
-; CHECK-SAME: (i32 [[NOT_GTID:%.*]], i32 [[GTID:%.*]])
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:[[C:%.*]] = icmp eq i32 [[GTID]], [[GTID]]
-; CHECK-NEXT:br i1 [[C]], label [[T:%.*]], label [[E:%.*]]
-; CHECK:   t:
-; CHECK-NEXT:call void @useI32(i32 [[GTID]])
-; CHECK-NEXT:call void @external(i1 [[C]])
-; CHECK-NEXT:br label [[M:%.*]]
-; CHECK:   e:
-; CHECK-NEXT:call void @useI32(i32 [[GTID]])
-; CHECK-NEXT:br label [[M]]
-; CHECK:   m:
-; CHECK-NEXT:call void @useI32(i32 [[GTID]])
-; CHECK-NEXT:ret void
-;
-entry:
-  %cc = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  %c = icmp eq i32 %cc, %gtid
-  br i1 %c, label %t, label %e
-t:
-  %c0 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @useI32(i32 %c0)
-  call void @external(i1 %c)
-  br label %m
-e:
-  %c1 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @useI32(i32 %c1)
-  br label %m
-m:
-  %c2 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0)
-  call void @useI32(i32 %c2)
-  ret void
-}
Index: llvm/test/Transforms/OpenMP/deduplication.ll
===
--- /dev/null
+++ llvm/test/Transforms/OpenMP/deduplication.ll
@@ -0,0 +1,223 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
+; RUN: opt -openmpopt -S < %s | FileCheck %s
+; RUN: opt -passes=openmpopt -S < %s | FileCheck %s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+%struct.ident_t = type { i32, i32, i32, i32, i8* }
+
+@0 = private unnamed_addr global %struct.ident_t { i32 0, i32 34, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str0, i32 0, i32 0) }, align 8
+@1 = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str1, i32 0, i32 0) }, align 8
+@2 = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds 

[PATCH] D74925: [OpenMP][Opt] Combine `struct ident_t*` during deduplication

2020-02-24 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.



In D74925#1889185 , @JonChesterfield 
wrote:

> Cool. Can we reasonably add the reproducible from 44893 to a regression 
> suite, in addition to these IR tests? It's written in C so would need to be 
> under clang's tests.


Done.

> This looks like a good fix for the reported bug. I don't see why this failure 
> mode would be unique to ident_t though, at least from re-reading 
> deduplicateRuntimeCalls - is it a feature of the current set of runtime calls 
> that can be deduplicated?

Good catch. Added enough code to be future proof wrt. miscompiles. Also added 
an extra test in IR.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:191
 
-private:
+  /// Return the insertion point used by the underlying IRBuilder.
+  InsertPointTy getInsertionPoint() { return Builder.saveIP(); }

JonChesterfield wrote:
> Probably don't want to drop the `private` annotation here
I actually do. The new and some of the below functions should be exposed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74925



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