From dc3b5b00fea104f231ae70fecab9d625fe14158c Mon Sep 17 00:00:00 2001
From: Johannes Doerfert <johannes@jdoerfert.de>
Date: Mon, 20 Apr 2020 18:25:24 -0500
Subject: [PATCH] [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.
---
 llvm/lib/Transforms/IPO/OpenMPOpt.cpp        | 22 +++++++++++++---------
 llvm/test/Transforms/OpenMP/deduplication.ll |  2 +-
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
index eb976ef..94c027c 100644
--- a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -235,14 +235,17 @@ private:
     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 @@ private:
   /// 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 afd22d5..a25d980 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]])
-- 
2.7.4