[llvm] dc3b5b0 - [OpenMPOpt] Make the combination of `ident_t*` deterministic

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2020-04-20T23:27:08-05:00
New Revision: dc3b5b00fea104f231ae70fecab9d625fe14158c

URL: https://github.com/llvm/llvm-project/commit/dc3b5b00fea104f231ae70fecab9d625fe14158c
DIFF: https://github.com/llvm/llvm-project/commit/dc3b5b00fea104f231ae70fecab9d625fe14158c.diff

LOG: [OpenMPOpt] Make the combination of `ident_t*` deterministic

Before we kept the first applicable `ident_t*` during deduplication of
runtime calls. The problem is that "first" is dependent on the iteration
order of a DenseMap. Since the proper solution, which is to combine the
information from all `ident_t*`, should be deterministic on its own, we
will not try to make the iteration order deterministic. Instead, we will
create a fresh `ident_t*` if there is not a unique existing `ident_t*`
to pick.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/OpenMPOpt.cpp
    llvm/test/Transforms/OpenMP/deduplication.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
index eb976efa512f..94c027cb7e54 100644
--- a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -235,14 +235,17 @@ struct OpenMPOpt {
     return Changed;
   }
 
-  static Value *combinedIdentStruct(Value *Ident0, Value *Ident1,
-                                    bool GlobalOnly) {
+  static Value *combinedIdentStruct(Value *CurrentIdent, Value *NextIdent,
+                                    bool GlobalOnly, bool &SingleChoice) {
+    if (CurrentIdent == NextIdent)
+      return CurrentIdent;
+
     // TODO: Figure out how to actually combine multiple debug locations. For
-    //       now we just keep the first we find.
-    if (Ident0)
-      return Ident0;
-    if (!GlobalOnly || isa<GlobalValue>(Ident1))
-      return Ident1;
+    //       now we just keep an existing one if there is a single choice.
+    if (!GlobalOnly || isa<GlobalValue>(NextIdent)) {
+      SingleChoice = !CurrentIdent;
+      return NextIdent;
+    }
     return nullptr;
   }
 
@@ -253,18 +256,19 @@ struct OpenMPOpt {
   /// information, e.g., the source locations, see combinedIdentStruct.
   Value *getCombinedIdentFromCallUsesIn(RuntimeFunctionInfo &RFI, Function &F,
                                         bool GlobalOnly) {
+    bool SingleChoice = true;
     Value *Ident = nullptr;
     auto CombineIdentStruct = [&](Use &U, Function &Caller) {
       CallInst *CI = getCallIfRegularCall(U, &RFI);
       if (!CI || &F != &Caller)
         return false;
       Ident = combinedIdentStruct(Ident, CI->getArgOperand(0),
-                                  /* GlobalOnly */ true);
+                                  /* GlobalOnly */ true, SingleChoice);
       return false;
     };
     RFI.foreachUse(CombineIdentStruct);
 
-    if (!Ident) {
+    if (!Ident || !SingleChoice) {
       // The IRBuilder uses the insertion block to get to the module, this is
       // unfortunate but we work around it for now.
       if (!OMPBuilder.getInsertionPoint().getBlock())

diff  --git a/llvm/test/Transforms/OpenMP/deduplication.ll b/llvm/test/Transforms/OpenMP/deduplication.ll
index afd22d522092..a25d980b1806 100644
--- a/llvm/test/Transforms/OpenMP/deduplication.ll
+++ b/llvm/test/Transforms/OpenMP/deduplication.ll
@@ -104,7 +104,7 @@ m:
 define void @local_and_global_gtid_calls() {
 ; CHECK-LABEL: define {{[^@]+}}@local_and_global_gtid_calls()
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TID5:%.*]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @2)
+; CHECK-NEXT:    [[TID5:%.*]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @3)
 ; CHECK-NEXT:    [[DOTKMPC_LOC_ADDR:%.*]] = alloca [[STRUCT_IDENT_T:%.*]], align 8
 ; CHECK-NEXT:    call void @useI32(i32 [[TID5]])
 ; CHECK-NEXT:    call void @useI32(i32 [[TID5]])