[IRMover] Set Address Space for moved global values

[ekatz]

Summary

Set Address Space when creating a new function (from another).

Fix PR41154.

[tejohnson]

This seems fine to me, but can you pull out and commit the NFC parts first, and update the patch to just contain the functional change? It's harder to see with all the NFC changes in here. The NFC part can go in without a phabricator patch.

llvm/test/Linker/addrspace.ll

1

suggest adding an alias in here for completeness

[ekatz]

Added alias testing, and moved the NFC part into https://reviews.llvm.org/D69550.

[tejohnson]

LGTM

[ekatz]

I cannot commit this patch myself.
Please commit it, if you may, in my behalf.

Author: Ehud Katz <[e-mail]>

Thanks.

llvm/lib/Linker/IRMover.cpp

GlobalVariable *IRLinker::copyGlobalVariableProto(const GlobalVariable *SGVar) {
   // No linking to be performed or linking from the source: simply create an
   // identical version of the symbol over in the dest module... the
   // initializer will be filled in later by LinkGlobalInits.
   GlobalVariable *NewDGV =
       new GlobalVariable(DstM, TypeMap.get(SGVar->getValueType()),
                          SGVar->isConstant(), GlobalValue::ExternalLinkage,
                          /*init*/ nullptr, SGVar->getName(),
                          /*insertbefore*/ nullptr, SGVar->getThreadLocalMode(),
-                         SGVar->getType()->getAddressSpace());
+                         SGVar->getAddressSpace());
   NewDGV->setAlignment(MaybeAlign(SGVar->getAlignment()));
   NewDGV->copyAttributesFrom(SGVar);
   return NewDGV;
 }
 
 AttributeList IRLinker::mapAttributeTypes(LLVMContext &C, AttributeList Attrs) {
   for (unsigned i = 0; i < Attrs.getNumAttrSets(); ++i) {
     if (Attrs.hasAttribute(i, Attribute::ByVal)) {
AttributeList IRLinker::mapAttributeTypes(LLVMContext &C, AttributeList Attrs) {
   return Attrs;
 }
 
 /// Link the function in the source module into the destination module if
 /// needed, setting up mapping information.
 Function *IRLinker::copyFunctionProto(const Function *SF) {
   // If there is no linkage to be performed or we are linking from the source,
   // bring SF over.
-  auto *F =
-      Function::Create(TypeMap.get(SF->getFunctionType()),
-                       GlobalValue::ExternalLinkage, SF->getName(), &DstM);
+  auto *F = Function::Create(TypeMap.get(SF->getFunctionType()),
+                             GlobalValue::ExternalLinkage,
+                             SF->getAddressSpace(), SF->getName(), &DstM);
   F->copyAttributesFrom(SF);
   F->setAttributes(mapAttributeTypes(F->getContext(), F->getAttributes()));
   return F;
 }
 
 /// Set up prototypes for any indirect symbols that come over from the source
 /// module.
 GlobalValue *
 IRLinker::copyGlobalIndirectSymbolProto(const GlobalIndirectSymbol *SGIS) {
   // If there is no linkage to be performed or we're linking from the source,
   // bring over SGA.
   auto *Ty = TypeMap.get(SGIS->getValueType());
   GlobalIndirectSymbol *GIS;
   if (isa<GlobalAlias>(SGIS))
-    GIS = GlobalAlias::create(Ty, SGIS->getType()->getPointerAddressSpace(),
+    GIS = GlobalAlias::create(Ty, SGIS->getAddressSpace(),
                               GlobalValue::ExternalLinkage, SGIS->getName(),
                               &DstM);
   else
-    GIS = GlobalIFunc::create(Ty, SGIS->getType()->getPointerAddressSpace(),
+    GIS = GlobalIFunc::create(Ty, SGIS->getAddressSpace(),
                               GlobalValue::ExternalLinkage, SGIS->getName(),
                               nullptr, &DstM);
   GIS->copyAttributesFrom(SGIS);
   return GIS;
 }
 
 GlobalValue *IRLinker::copyGlobalValueProto(const GlobalValue *SGV,
                                             bool ForDefinition) {
   GlobalValue *NewGV;
   if (auto *SGVar = dyn_cast<GlobalVariable>(SGV)) {
     NewGV = copyGlobalVariableProto(SGVar);
   } else if (auto *SF = dyn_cast<Function>(SGV)) {
     NewGV = copyFunctionProto(SF);
   } else {
     if (ForDefinition)
       NewGV = copyGlobalIndirectSymbolProto(cast<GlobalIndirectSymbol>(SGV));
     else if (SGV->getValueType()->isFunctionTy())
       NewGV =
           Function::Create(cast<FunctionType>(TypeMap.get(SGV->getValueType())),
-                           GlobalValue::ExternalLinkage, SGV->getName(), &DstM);
+                           GlobalValue::ExternalLinkage, SGV->getAddressSpace(),
+                           SGV->getName(), &DstM);
     else
-      NewGV = new GlobalVariable(
-          DstM, TypeMap.get(SGV->getValueType()),
+      NewGV =
+          new GlobalVariable(DstM, TypeMap.get(SGV->getValueType()),
                              /*isConstant*/ false, GlobalValue::ExternalLinkage,
                              /*init*/ nullptr, SGV->getName(),
-          /*insertbefore*/ nullptr, SGV->getThreadLocalMode(),
-          SGV->getType()->getAddressSpace());
+                             /*insertbefore*/ nullptr,
+                             SGV->getThreadLocalMode(), SGV->getAddressSpace());
   }
 
   if (ForDefinition)
     NewGV->setLinkage(SGV->getLinkage());
   else if (SGV->hasExternalWeakLinkage())
     NewGV->setLinkage(GlobalValue::ExternalWeakLinkage);
 
   if (auto *NewGO = dyn_cast<GlobalObject>(NewGV)) {
IRLinker::linkAppendingVarProto(GlobalVariable *DstGV,
   }
   uint64_t NewSize = DstNumElements + SrcElements.size();
   ArrayType *NewType = ArrayType::get(EltTy, NewSize);
 
   // Create the new global variable.
   GlobalVariable *NG = new GlobalVariable(
       DstM, NewType, SrcGV->isConstant(), SrcGV->getLinkage(),
       /*init*/ nullptr, /*name*/ "", DstGV, SrcGV->getThreadLocalMode(),
-      SrcGV->getType()->getAddressSpace());
+      SrcGV->getAddressSpace());
 
   NG->copyAttributesFrom(SrcGV);
   forceRenaming(NG, SrcGV->getName());
 
   Constant *Ret = ConstantExpr::getBitCast(NG, TypeMap.get(SrcGV->getType()));
 
   Mapper.scheduleMapAppendingVariable(*NG,
                                       DstGV ? DstGV->getInitializer() : nullptr,

llvm/test/Linker/addrspace.ll

+; RUN: llvm-link %s -S | FileCheck %s
+
+@G = addrspace(2) global i32 256 
+; CHECK: @G = addrspace(2) global i32
+
+define void @foo() addrspace(3) {
+; CHECK: define void @foo() addrspace(3)
+  ret void
+}