bridge: improve code generation in non-integral pointer mode

Previously, when using non-integral pointers, for local
variables we create allocas in address space 0 and immediately
cast to address space 1. The addrspacecast instructions we
created does not play well with the optimizer (it works
correctly, but much conservative). Also, this makes the debug
info really bad (essentially all the locals are lost).

Observe that loads and stores don't really care the address
space of the pointer. The only time where the address space
matters is when we take the address. Instead of creating
addrspacecast instructions at the time we create the alloca,
emit it only when taking the address.

This way, most of the non-address-taken allocas will not have
addrspacecast, so it can be SSA'd. The address-taken ones cannot
be SSA'd anyway, so the addrspacecast doesn't hurt much.

As the addrspacecast instruction generation is delayed, it makes
the deferreturn sequence slightly more complicated, as there is
now dependency between the instructions in the sequence. Handle
this in the instruction cloning code in genDeferReturn.

Adjust the call sequence generation as the temporaries for
passing the arguments and results are now in address space 0.

Change-Id: I8400fc4c0b19b6038b70a6949eb2447bc3202582
Reviewed-on: https://go-review.googlesource.com/c/156677
Reviewed-by: Than McIntosh <thanm@google.com>
diff --git a/bridge/go-llvm-bfunction.cpp b/bridge/go-llvm-bfunction.cpp
index 216214c..7109ccb 100644
--- a/bridge/go-llvm-bfunction.cpp
+++ b/bridge/go-llvm-bfunction.cpp
@@ -81,19 +81,6 @@
   if (! name.empty())
     inst->setName(name);
   allocas_.push_back(inst);
-
-  // Immediately cast to address space
-  unsigned addressSpace = abiOracle_->tm()->addressSpace();
-  if (addressSpace != 0) {
-    llvm::Type *pt = llvm::PointerType::get(typ, addressSpace);
-    inst = new llvm::AddrSpaceCastInst(inst, pt);
-    if (! name.empty()) {
-      std::string castname(NameGen::combineTags(name, ".ascast"));
-      inst->setName(namegen(castname));
-    }
-    allocas_.push_back(inst);
-  }
-
   return inst;
 }
 
@@ -377,7 +364,7 @@
     if (paramInfo.abiType()->isVectorTy() ||
         arg->getType() != llpt->getElementType()) {
       std::string tag(namegen("cast"));
-      llvm::Type *ptv = tm->makeLLVMPointerType(paramInfo.abiType());
+      llvm::Type *ptv = llvm::PointerType::get(paramInfo.abiType(), 0);
       llvm::Value *bitcast = builder.CreateBitCast(sploc, ptv, tag);
       sploc = bitcast;
     }
@@ -391,7 +378,7 @@
 
   // Create struct type corresponding to first and second params
   llvm::Type *llst = paramInfo.computeABIStructType(tm);
-  llvm::Type *ptst = tm->makeLLVMPointerType(llst);
+  llvm::Type *ptst = llvm::PointerType::get(llst, 0);
 
   // Cast the spill location to a pointer to the struct created above.
   std::string tag(namegen("cast"));
@@ -545,7 +532,7 @@
   llvm::Type *llrt = (returnInfo.abiType()->isAggregateType() ?
                       returnInfo.computeABIStructType(tm) :
                       returnInfo.abiType());
-  llvm::Type *ptst = tm->makeLLVMPointerType(llrt);
+  llvm::Type *ptst = llvm::PointerType::get(llrt, 0);
   BinstructionsLIRBuilder builder(function()->getContext(), retInstrs);
   std::string castname(namegen("cast"));
   llvm::Value *bitcast = builder.CreateBitCast(toRet->value(), ptst, castname);
diff --git a/bridge/go-llvm-bnode.cpp b/bridge/go-llvm-bnode.cpp
index 9e6fd9a..a3a1ac9 100644
--- a/bridge/go-llvm-bnode.cpp
+++ b/bridge/go-llvm-bnode.cpp
@@ -523,12 +523,6 @@
   if (! name.empty())
     inst->setName(name);
 
-  // Immediately cast to address space
-  if (addressSpace_ != 0) {
-    llvm::Type *pt = llvm::PointerType::get(typ, addressSpace_);
-    inst = new llvm::AddrSpaceCastInst(inst, pt);
-  }
-
   Bvariable *tvar = new Bvariable(varType, loc, name, LocalVar, true, inst);
   tempvars_[inst] = tvar;
   tvar->markAsTemporary();
@@ -625,6 +619,7 @@
 {
   std::vector<Bnode *> kids = { src };
   Bexpression *rval = new Bexpression(N_Address, kids, val, typ, loc);
+  appendInstIfNeeded(rval, val);
   return archive(rval);
 }
 
diff --git a/bridge/go-llvm-materialize.cpp b/bridge/go-llvm-materialize.cpp
index 1f9c669..920bae5 100644
--- a/bridge/go-llvm-materialize.cpp
+++ b/bridge/go-llvm-materialize.cpp
@@ -106,6 +106,19 @@
     val = cv->value();
   }
 
+  // When using non-integral pointers, the Go pointer types (Btype)
+  // are in address space 1. Local variables (allocas) are always
+  // in address space 0. We need to insert a cast if we are taking
+  // the address of a local variable.
+  llvm::Type *valtyp = val->getType();
+  assert(valtyp->isPointerTy());
+  if (valtyp->getPointerAddressSpace() != addressSpace_) {
+    llvm::Type *typ =
+        llvm::PointerType::get(valtyp->getPointerElementType(),
+                               addressSpace_);
+    val = new llvm::AddrSpaceCastInst(val, typ, "ascast");
+  }
+
   // Create new expression with proper type.
   Btype *pt = pointer_type(bexpr->btype());
   Bexpression *rval = nbuilder_.mkAddress(pt, val, bexpr, location);
@@ -1116,8 +1129,16 @@
     Btype *resTyp = state.calleeFcnType->resultType();
     assert(state.callerFcn);
     state.sretTemp = state.callerFcn->createTemporary(resTyp, tname);
-    if (returnInfo.disp() == ParmIndirect)
-      state.llargs.push_back(state.sretTemp);
+    if (returnInfo.disp() == ParmIndirect) {
+      llvm::Value *sretval = state.sretTemp;
+      if (addressSpace_ != 0) {
+        llvm::Type *typ =
+            makeLLVMPointerType(sretval->getType()->getPointerElementType());
+        sretval =
+            state.builder.CreateAddrSpaceCast(sretval, typ, "sret.actual.ascast");
+      }
+      state.llargs.push_back(sretval);
+    }
   }
 
   // Chain param if needed
@@ -1197,7 +1218,12 @@
           val = cv->value();
         }
         std::string castname(namegen("cast"));
-        llvm::Type *ptv = makeLLVMPointerType(paramInfo.abiType());
+        // We are going to do a load, so the address space does not matter.
+        // It seems we may get here with either address space, so we just
+        // do an address-space-preserving cast.
+        llvm::Type *ptv =
+            llvm::PointerType::get(paramInfo.abiType(),
+                                   val->getType()->getPointerAddressSpace());
         llvm::Value *bitcast = builder.CreateBitCast(val, ptv, castname);
         std::string ltag(namegen("ld"));
         llvm::Value *ld = builder.CreateLoad(bitcast, ltag);
@@ -1322,7 +1348,7 @@
       llvm::Type *rt = (returnInfo.abiTypes().size() == 1 ?
                         returnInfo.abiType()  :
                         returnInfo.computeABIStructType(typeManager()));
-      llvm::Type *ptrt = makeLLVMPointerType(rt);
+      llvm::Type *ptrt = llvm::PointerType::get(rt, 0);
       std::string castname(namegen("cast"));
       llvm::Value *bitcast = builder.CreateBitCast(state.sretTemp,
                                                    ptrt, castname);
diff --git a/bridge/go-llvm.cpp b/bridge/go-llvm.cpp
index 963c042..0d605cb 100644
--- a/bridge/go-llvm.cpp
+++ b/bridge/go-llvm.cpp
@@ -3290,6 +3290,23 @@
   return nullptr;
 }
 
+// A helper function that clones an instruction, and fixes up the operands
+// if they are previously cloned values.
+static llvm::Instruction *
+cloneInstruction(llvm::Instruction *inst,
+                 std::unordered_map<llvm::Value*, llvm::Value*> &argMap)
+{
+  llvm::Instruction *c = inst->clone();
+  assert(argMap.find(inst) == argMap.end());
+  argMap[inst] = c;
+  for (unsigned i = 0, n = c->getNumOperands(); i < n; i++) {
+    auto it = argMap.find(c->getOperand(i));
+    if (it != argMap.end())
+      c->setOperand(i, it->second);
+  }
+  return c;
+}
+
 // Generate the DEFERRETURN call for return statement in functions
 // that has defer.
 // We could simply rewrite the return to a jump into the 'finally'
@@ -3303,17 +3320,21 @@
 {
   llvm::BasicBlock *blockToClone = finallyBlock_;
   assert(blockToClone);
+
+  // A map from old value to new value, used to fix up operands.
+  std::unordered_map<llvm::Value*, llvm::Value*> argMap;
+
   while (1) {
     llvm::Instruction *term = blockToClone->getTerminator();
     for (llvm::Instruction &inst : *blockToClone) {
       if (&inst == term)
         break; // terminator is handled below
-      llvm::Instruction *c = inst.clone();
+      llvm::Instruction *c = cloneInstruction(&inst, argMap);
       curblock->getInstList().push_back(c);
     }
     if (llvm::isa<llvm::InvokeInst>(term)) {
       // This is the call to DEFERRETURN. Copy this then we're done.
-      llvm::Instruction *c = term->clone();
+      llvm::Instruction *c = cloneInstruction(term, argMap);
       curblock->getInstList().push_back(c);
       break;
     }