From 7e0629a1d8cc6f2cbc9858c1b91c787087e64172 Mon Sep 17 00:00:00 2001
From: Keno Fischer <kfischer@college.harvard.edu>
Date: Wed, 16 Dec 2015 13:36:39 +0100
Subject: [PATCH] Address review comments

---
 src/cgutils.cpp    | 23 +++++++++++++----------
 src/intrinsics.cpp |  2 +-
 src/jitlayers.cpp  | 14 +++++++++-----
 src/threading.c    |  2 +-
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/src/cgutils.cpp b/src/cgutils.cpp
index d2fae5c3d7365..6e0765704a039 100644
--- a/src/cgutils.cpp
+++ b/src/cgutils.cpp
@@ -29,6 +29,9 @@ static Instruction *tbaa_decorate(MDNode* md, Instruction* load_or_store)
     return load_or_store;
 }
 
+// This is the same as the IRBUilder's CreateGlobalString(Ptr), except that the
+// string constant gets created in the active module rather than the function's
+// parent module.
 static GlobalVariable *CreateGlobalString(StringRef Str,
                                                   const Twine &Name,
                                                   unsigned AddressSpace) {
@@ -42,8 +45,6 @@ static GlobalVariable *CreateGlobalString(StringRef Str,
     return GV;
 }
 
-/// \brief Same as CreateGlobalString, but return a pointer with "i8*" type
-/// instead of a pointer to array of i8.
 static Value *CreateGlobalStringPtr(llvm::IRBuilder<> *Builder, StringRef Str, const Twine &Name = "",
                            unsigned AddressSpace = 0) {
     GlobalVariable *gv = CreateGlobalString(Str, Name, AddressSpace);
@@ -154,14 +155,14 @@ static bool handleUse(Use &Use1,llvm::GlobalValue *G,std::map<llvm::Module*,llvm
         Value::use_iterator UI2 = Expr->use_begin(), E2 = Expr->use_end();
         for (; UI2 != E2;) {
             Use &Use2 = *UI2;
-             ++UI2;
-             struct ExprChain NextChain;
-             NextChain.Expr = Expr;
-             NextChain.OpNo = Use1.getOperandNo();
-             NextChain.Next = nullptr;
-             if (ChainEnd)
-                ChainEnd->Next = &NextChain;
-             handleUse(Use2,G,FixedGlobals,Chain ? Chain : &NextChain,&NextChain);
+            ++UI2;
+            struct ExprChain NextChain;
+            NextChain.Expr = Expr;
+            NextChain.OpNo = Use1.getOperandNo();
+            NextChain.Next = nullptr;
+            if (ChainEnd)
+               ChainEnd->Next = &NextChain;
+            handleUse(Use2,G,FixedGlobals,Chain ? Chain : &NextChain,&NextChain);
         }
         return true;
     }
@@ -455,9 +456,11 @@ class FunctionMover : public ValueMaterializer
                     if (oldF)
                         return oldF;
 
+#ifndef USE_ORCJIT
                     // Also check if this function is pending in any other module
                     if (jl_ExecutionEngine->FindFunctionNamed(F->getName().data()))
                         return InjectFunctionProto(F);
+#endif
 
                     return CloneFunctionProto(shadow);
                 }
diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp
index 2fce29bffd522..6e807847290fb 100644
--- a/src/intrinsics.cpp
+++ b/src/intrinsics.cpp
@@ -1156,7 +1156,7 @@ static Value *emit_untyped_intrinsic(intrinsic f, Value *x, Value *y, Value *z,
 #ifdef LLVM37
       return builder.CreateCall(prepare_call(fmaintr),{ FP(x), FP(y), FP(z) });
 #else
-      return builder.CreateCall3(fmaintr, FP(x), FP(y), FP(z));
+      return builder.CreateCall3(prepare_call(fmaintr), FP(x), FP(y), FP(z));
 #endif
     }
     case muladd_float:
diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp
index 5295c6073daa5..b8e0a602faf6f 100644
--- a/src/jitlayers.cpp
+++ b/src/jitlayers.cpp
@@ -226,8 +226,9 @@ class DebugObjectRegistrar {
                 auto NewObj = ObjectFile::createObjectFile(NewBuffer->getMemBufferRef());
                 SavedObject = OwningBinary<ObjectFile>(std::move(*NewObj),std::move(NewBuffer));
             }
-            else
+            else {
                 NotifyGDB(SavedObject);
+            }
 
             SavedObjects.push_back(std::move(SavedObject));
             ORCNotifyObjectEmitted(JuliaListener.get(),*Object,*SavedObjects.back().getBinary(),*LO);
@@ -286,10 +287,13 @@ class JuliaOJIT {
                     ErrorOr<std::unique_ptr<object::ObjectFile>> Obj =
                         object::ObjectFile::createObjectFile(ObjBuffer->getMemBufferRef());
 
-                    // TODO: Actually report errors helpfully.
-                    if (Obj)
-                        return OwningObj(std::move(*Obj), std::move(ObjBuffer));
-                    return OwningObj(nullptr, nullptr);
+                    if (!Obj) {
+                        M.dump();
+                        llvm::report_fatal_error("FATAL: Unable to compile LLVM Module.\n"
+                            "The module's content was printed above. Please file a bug report");
+                    }
+
+                    return OwningObj(std::move(*Obj), std::move(ObjBuffer));
                 }
             )};
             // Make sure SectionMemoryManager::getSymbolAddressInProcess can resolve
diff --git a/src/threading.c b/src/threading.c
index 52578d911cbf8..445599c866a34 100644
--- a/src/threading.c
+++ b/src/threading.c
@@ -421,7 +421,7 @@ JL_DLLEXPORT jl_value_t *jl_threading_run(jl_function_t *f, jl_svec_t *args)
         argtypes = (jl_tupletype_t*)jl_typeof(jl_emptytuple);
     else
         argtypes = arg_type_tuple(jl_svec_data(args), jl_svec_len(args));
-    fun = jl_get_specialization(f, argtypes);
+    fun = jl_get_specialization(f, argtypes, NULL);
     if (fun == NULL)
         fun = f;
     jl_generate_fptr(fun);