Address Further ORC review comments

src/ccall.cpp

@@ -13,6 +13,17 @@ static std::set<u_int64_t> llvmcallDecls;
 static std::map<std::string, GlobalVariable*> libMapGV;
 static std::map<std::string, GlobalVariable*> symMapGV;
 
+static Value *GetStringPtr(llvm::IRBuilder <> *builder, GlobalValue *GV, const Twine &Name)
+{
+ Value *zero = ConstantInt::get(Type::getInt32Ty(jl_LLVMContext), 0);
+ Value *Args[] = { zero, zero };
+#ifdef LLVM37
+ return builder->CreateInBoundsGEP(GV->getValueType(), GV, Args, Name);
+#else
+ return builder->CreateInBoundsGEP(GV, Args, Name);
+#endif
+}
+
 static Value *runtime_sym_lookup(PointerType *funcptype, const char *f_lib, const char *f_name, jl_codectx_t *ctx)
 {
  // in pseudo-code, this function emits the following:
@@ -94,15 +105,15 @@ static Value *runtime_sym_lookup(PointerType *funcptype, const char *f_lib, cons
  builder.SetInsertPoint(dlsym_lookup);
  Value *libname;
  if (runtime_lib) {
- libname = CreateGlobalStringPtr(&builder,f_lib, "f_lib");
+ libname = GetStringPtr(&builder, stringConst(f_lib), "f_lib");
  }
  else {
  libname = literal_static_pointer_val(f_lib, T_pint8);
  }
 #ifdef LLVM37
- Value *llvmf = builder.CreateCall(prepare_call(jldlsym_func), { libname, CreateGlobalStringPtr(&builder, f_name, "f_name"), libptrgv });
+ Value *llvmf = builder.CreateCall(prepare_call(jldlsym_func), { libname, GetStringPtr(&builder, stringConst(f_name), "f_name"), libptrgv });
 #else
- Value *llvmf = builder.CreateCall3(prepare_call(jldlsym_func), libname, CreateGlobalStringPtr(&builder, f_name, "f_name"), libptrgv);
+ Value *llvmf = builder.CreateCall3(prepare_call(jldlsym_func), libname, GetStringPtr(&builder, stringConst(f_name), "f_name"), libptrgv);
 #endif
  builder.CreateStore(llvmf, llvmgv);
  builder.CreateBr(ccall_bb);

src/cgutils.cpp

@@ -29,35 +29,6 @@ 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) {
- Constant *StrConstant = ConstantDataArray::getString(jl_LLVMContext, Str);
- GlobalVariable *GV = new GlobalVariable(*active_module, StrConstant->getType(),
- true, GlobalValue::PrivateLinkage,
- StrConstant, Name, NULL,
- GlobalVariable::NotThreadLocal,
- AddressSpace);
- GV->setUnnamedAddr(true);
- return GV;
-}
-
-static Value *CreateGlobalStringPtr(llvm::IRBuilder<> *Builder, StringRef Str, const Twine &Name = "",
- unsigned AddressSpace = 0) {
- GlobalVariable *gv = CreateGlobalString(Str, Name, AddressSpace);
- Value *zero = ConstantInt::get(Type::getInt32Ty(jl_LLVMContext), 0);
- Value *Args[] = { zero, zero };
-#ifdef LLVM37
- return Builder->CreateInBoundsGEP(gv->getValueType(), gv, Args, Name);
-#else
- return Builder->CreateInBoundsGEP(gv, Args, Name);
-#endif
-}
-
-
 // Fixing up references to other modules for MCJIT
 std::set<llvm::GlobalValue*> pending_globals;
 static GlobalVariable *prepare_global(GlobalVariable *G)
@@ -98,7 +69,9 @@ static GlobalValue *realize_pending_global(Instruction *User, GlobalValue *G, st
  if (!NewGV) {
  NewGV = new GlobalVariable(*M, GV->getType()->getElementType(),
  GV->isConstant(), GlobalVariable::ExternalLinkage,
- NULL, GV->getName(), NULL, GV->getThreadLocalMode());
+ NULL, GV->getName(), NULL, GV->getThreadLocalMode(),
+ GV->getType()->getAddressSpace());
+ NewGV->setUnnamedAddr(GV->hasUnnamedAddr());
  // Move over initializer
  if (GV->hasInitializer()) {
  NewGV->setInitializer(GV->getInitializer());
@@ -146,12 +119,11 @@ struct ExprChain {
  struct ExprChain *Next;
 };
 
-static bool handleUse(Use &Use1,llvm::GlobalValue *G,std::map<llvm::Module*,llvm::GlobalValue*> &FixedGlobals,struct ExprChain *Chain, struct ExprChain *ChainEnd)
+static void handleUse(Use &Use1,llvm::GlobalValue *G,std::map<llvm::Module*,llvm::GlobalValue*> &FixedGlobals,struct ExprChain *Chain, struct ExprChain *ChainEnd)
 {
  Instruction *User = dyn_cast<Instruction>(Use1.getUser());
  if (!User) {
- ConstantExpr *Expr = dyn_cast<ConstantExpr>(Use1.getUser());
- assert(Expr);
+ ConstantExpr *Expr = cast<ConstantExpr>(Use1.getUser());
  Value::use_iterator UI2 = Expr->use_begin(), E2 = Expr->use_end();
  for (; UI2 != E2;) {
  Use &Use2 = *UI2;
@@ -164,17 +136,17 @@ static bool handleUse(Use &Use1,llvm::GlobalValue *G,std::map<llvm::Module*,llvm
  ChainEnd->Next = &NextChain;
  handleUse(Use2,G,FixedGlobals,Chain ? Chain : &NextChain,&NextChain);
  }
- return true;
+ return;
  }
  llvm::Constant *Replacement = realize_pending_global(User,G,FixedGlobals);
  if (!Replacement)
- return false;
+ return;
  while (Chain) {
  Replacement = Chain->Expr->getWithOperandReplaced(Chain->OpNo,Replacement);
+ Chain->Expr = cast<ConstantExpr>(Replacement);
  Chain = Chain->Next;
  }
  Use1.set(Replacement);
- return true;
 }
 
 // RAUW, but only for those users which live in a module, and create a module
@@ -183,12 +155,12 @@ static void realize_pending_globals()
 {
  std::set<llvm::GlobalValue *> local_pending_globals;
  std::swap(local_pending_globals,pending_globals);
+ std::map<llvm::Module*,llvm::GlobalValue*> FixedGlobals;
  for (auto *G : local_pending_globals) {
- std::map<llvm::Module*,llvm::GlobalValue*> FixedGlobals;
  Value::use_iterator UI = G->use_begin(), E = G->use_end();
  for (; UI != E;)
- if (!handleUse(*(UI++),G,FixedGlobals,nullptr,nullptr))
-  continue;
+ handleUse(*(UI++),G,FixedGlobals,nullptr,nullptr);
+ FixedGlobals.clear();
  }
 }
 
@@ -332,24 +304,6 @@ static Function *function_proto(Function *F) {
  NewF->setPersonalityFn(nullptr);
 #endif
 
- AttributeSet OldAttrs = F->getAttributes();
- // Clone any argument attributes that are present in the VMap.
- auto ArgI = NewF->arg_begin();
- for (const Argument &OldArg : F->args()) {
- AttributeSet attrs =
- OldAttrs.getParamAttributes(OldArg.getArgNo() + 1);
- if (attrs.getNumSlots() > 0)
- ArgI->addAttr(attrs);
- ++ArgI;
- }
-
- NewF->setAttributes(
- NewF->getAttributes()
- .addAttributes(NewF->getContext(), AttributeSet::ReturnIndex,
- OldAttrs.getRetAttributes())
- .addAttributes(NewF->getContext(), AttributeSet::FunctionIndex,
- OldAttrs.getFnAttributes()));
-
  return NewF;
 }
 

src/intrinsics.cpp

@@ -1218,7 +1218,7 @@ static Value *emit_untyped_intrinsic(intrinsic f, Value *x, Value *y, Value *z,
 #ifdef LLVM37
  Value *res = builder.CreateCall(prepare_call(intr),{ix, iy});
 #else
- Value *res = builder.CreateCall2(intr, ix, iy);
+ Value *res = builder.CreateCall2(prepare_call(intr), ix, iy);
 #endif
  Value *obit = builder.CreateExtractValue(res, ArrayRef<unsigned>(1));
  raise_exception_if(obit, prepare_global(jlovferr_var), ctx);
@@ -1372,7 +1372,7 @@ static Value *emit_untyped_intrinsic(intrinsic f, Value *x, Value *y, Value *z,
 #ifdef LLVM37
  return builder.CreateCall(prepare_call(ctlz), {x, ConstantInt::get(T_int1,0)});
 #else
- return builder.CreateCall2(ctlz, x, ConstantInt::get(T_int1,0));
+ return builder.CreateCall2(prepare_call(ctlz), x, ConstantInt::get(T_int1,0));
 #endif
  }
  case cttz_int: {
@@ -1382,7 +1382,7 @@ static Value *emit_untyped_intrinsic(intrinsic f, Value *x, Value *y, Value *z,
 #ifdef LLVM37
  return builder.CreateCall(prepare_call(cttz), {x, ConstantInt::get(T_int1, 0)});
 #else
- return builder.CreateCall2(cttz, x, ConstantInt::get(T_int1, 0));
+ return builder.CreateCall2(prepare_call(cttz), x, ConstantInt::get(T_int1, 0));
 #endif
  }