gollvm: rework circular pointer type handing

Following CL 123796, use the concrete type passed by the frontend
for circular pointer types, instead of inferring it ourselves
based on the ordering of a few function calls. Now that we
record the actual concrete element type in the circular pointer
marker type's toType, some routines can be simplified.

Change circularConversionLoadMap_ and circularConversionAddrMap_
to keyed on LLVM types, since LLVM types are unique.

Adjust the tests to reflect the new way of API calls by the
frontend.

TODO: currently there is no test for the quality of the debug
info. Most of the existing tests are only check that we can
generate debug info (without crash) and they are legal. I
manually checked a few programs with circular types, and the
debug info looks ok to me. The circular type has its concrete
representation in debug info, with the second appearance
represented as *void. For example, "type T *[1]*T" is represented
as *[1]*(*void).

Change-Id: I360aeced5ebfd724dec0818c5e1c0ca77ab778bb
Reviewed-on: https://go-review.googlesource.com/124704
Reviewed-by: Than McIntosh <thanm@google.com>
diff --git a/bridge/go-llvm-materialize.cpp b/bridge/go-llvm-materialize.cpp
index 41d2179..0119ff5 100644
--- a/bridge/go-llvm-materialize.cpp
+++ b/bridge/go-llvm-materialize.cpp
@@ -609,24 +609,21 @@
     return rval;
   }
 
-  // Case 4: circular pointer type (ex: type T *T; var p, q T; p == &q)
+  // Case 4: pointer type comparison
+  // We check that if both are pointer types and pointing to the same type,
+  // insert a cast (it doesn't matter we cast which one). This mostly needed
+  // for circular pointer types (ex: type T *T; var p, q T; p == &q), where
+  // the two sides have semantically identical types but with different
+  // representations (in this case, T vs. *T).
   if (leftType->isPointerTy() && rightType->isPointerTy()) {
     BPointerType *lbpt = left->btype()->castToBPointerType();
-    Btype *lctypconv = circularTypeAddrConversion(lbpt->toType());
-    if (lctypconv != nullptr) {
-      std::string tag(namegen("cast"));
-      BexprLIRBuilder builder(context_, left);
-      llvm::Value *bitcast = builder.CreateBitCast(leftVal, lctypconv->type(), tag);
-      rval.first = bitcast;
-    }
-
     BPointerType *rbpt = right->btype()->castToBPointerType();
-    Btype *rctypconv = circularTypeAddrConversion(rbpt->toType());
-    if (rctypconv != nullptr) {
-      std::string tag(namegen("cast"));
+    if (lbpt->toType()->type() == rbpt->toType()->type()) {
       BexprLIRBuilder builder(context_, right);
-      llvm::Value *bitcast = builder.CreateBitCast(rightVal, rctypconv->type(), tag);
+      std::string tag(namegen("cast"));
+      llvm::Value *bitcast = builder.CreateBitCast(rightVal, leftType, tag);
       rval.second = bitcast;
+      return rval;
     }
   }
 
diff --git a/bridge/go-llvm-typemanager.cpp b/bridge/go-llvm-typemanager.cpp
index 2a36d46..ee97280 100644
--- a/bridge/go-llvm-typemanager.cpp
+++ b/bridge/go-llvm-typemanager.cpp
@@ -1000,7 +1000,8 @@
       // Link the marker type with the concrete type.
       BPointerType *bpt = cpt->castToBPointerType();
       BPointerType *ttpt = to_type->castToBPointerType();
-      bpt->setToType(ttpt->toType());
+      Btype *elt = ttpt->toType();
+      bpt->setToType(elt);
 
       if (traceLevel() > 1) {
         std::cerr << "\n^ link circular pointer type "
@@ -1013,6 +1014,10 @@
         std::cerr << "redir: "; to_type->dump();
       }
 
+      // Circular pointer type handling
+      circularConversionLoadMap_[cpt->type()] = elt;
+      circularConversionAddrMap_[elt->type()] = cpt;
+
       return setPlaceholderPointerType(placeholder, cpt);
     }
   }
@@ -1051,44 +1056,6 @@
     postProcessResolvedPlaceholder(placeholder);
   }
 
-  // Circular pointer type handling
-
-  unsigned nct = circularPointerLoop_.size();
-  if (nct != 0) {
-    btpair p = circularPointerLoop_[0];
-    Btype *cplTT = p.second;
-    if (to_type == cplTT) {
-      // We've come to the end of a cycle of circular types (in the
-      // sense that we're back visiting the placeholder that initiated
-      // the loop). Record proper values for inserting conversions when
-      // we have operations on this this (load, addr).
-      btpair adPair = circularPointerLoop_[nct == 1 ? 0 : 1];
-      Btype *adConvPH = adPair.first;
-      Btype *adConvTT = adPair.second;
-      btpair loadPair = circularPointerLoop_.back();
-      Btype *loadConvTT = loadPair.second;
-      circularConversionLoadMap_[to_type] = loadConvTT;
-      circularConversionLoadMap_[placeholder] = loadConvTT;
-      circularConversionAddrMap_[adConvPH] = to_type;
-      circularConversionAddrMap_[adConvTT] = to_type;
-
-      if (traceLevel() > 1) {
-        std::cerr << "\n^ finished circular type loop of size " << nct
-                  << ": [" << ((void*) loadConvTT)
-                  << "," << ((void*) adConvPH)
-                  << "," << ((void*) adConvTT)
-                  << "] for to_type: [load, addrPH, addrTT]:\n";
-        loadConvTT->dump();
-        adConvPH->dump();
-        adConvTT->dump();
-      }
-      circularPointerLoop_.clear();
-    } else {
-      if (! isCircularFunctionType(placeholder))
-        circularPointerLoop_.push_back(std::make_pair(placeholder, to_type));
-    }
-  }
-
   return true;
 }
 
@@ -1266,8 +1233,6 @@
     // Set up to start tracking the types that will make up the
     // loop involved in the cycle.
     circularPointerTypes_.insert(circ_typ);
-    assert(circularPointerLoop_.size() == 0);
-    circularPointerLoop_.push_back(std::make_pair(placeholder, rval));
   }
 
   if (traceLevel() > 1) {
@@ -1313,12 +1278,12 @@
 }
 
 Btype *TypeManager::circularTypeLoadConversion(Btype *typ) {
-  auto it = circularConversionLoadMap_.find(typ);
+  auto it = circularConversionLoadMap_.find(typ->type());
   return it != circularConversionLoadMap_.end()  ? it->second : nullptr;
 }
 
 Btype *TypeManager::circularTypeAddrConversion(Btype *typ) {
-  auto it = circularConversionAddrMap_.find(typ);
+  auto it = circularConversionAddrMap_.find(typ->type());
   if (it != circularConversionAddrMap_.end())
     return it->second;
   return nullptr;
@@ -1799,49 +1764,6 @@
   return ss.str();
 }
 
-llvm::DIType *TypeManager::buildCircularPointerDIType(Btype *typ,
-                                                      DIBuildHelper &helper)
-{
-  auto rnit = revNames_.find(typ);
-  if (rnit != revNames_.end())
-    typ = rnit->second;
-
-  std::unordered_map<Btype *, llvm::DIType*> &typeCache = helper.typeCache();
-  auto tcit = typeCache.find(typ);
-  if (tcit != typeCache.end() && tcit->second != nullptr)
-    return tcit->second;
-
-  // Create replaceable placeholder
-  llvm::DIBuilder &dibuilder = helper.dibuilder();
-  unsigned tag = llvm::dwarf::DW_TAG_structure_type;
-  llvm::DIScope *scope = helper.moduleScope();
-  llvm::DIFile *file = helper.diFileFromLocation(typ->location());
-  unsigned lineNumber = helper.linemap()->location_line(typ->location());
-  llvm::DICompositeType *placeholder =
-      dibuilder.createReplaceableCompositeType(tag, typToString(typ),
-                                               scope, file, lineNumber);
-  typeCache[typ] = placeholder;
-
-  // Convert what it points to
-  Btype *toType = circularTypeLoadConversion(typ);
-  if (!toType)
-    toType = typ->castToBPointerType()->toType();
-  llvm::DIType *toDIType = buildCircularPointerDIType(toType, helper);
-
-  // Now create final pointer type.
-  uint64_t bits = datalayout_->getPointerSizeInBits();
-  llvm::DIType *result = dibuilder.createPointerType(toDIType, bits);
-
-  // Replace the temp
-  dibuilder.replaceTemporary(llvm::TempDIType(placeholder), result);
-
-  // Update cache
-  typeCache[typ] = result;
-
-  // Done.
-  return result;
-}
-
 llvm::DIType *TypeManager::buildStructDIType(BStructType *bst,
                                              DIBuildHelper &helper)
 {
@@ -1986,15 +1908,9 @@
       if (bpt->isPlaceholder())
         return buildDIType(pointerType(voidType()), helper);
 
-      // Special case for circular pointer types
-      auto cpit = circularPointerTypes_.find(typ->type());
-      if (cpit != circularPointerTypes_.end()) {
-        rval = buildCircularPointerDIType(typ, helper);
-        break;
-      }
       assert(bpt->toType() != nullptr);
 
-      // Similarly, if we're pointing to something unresolved, be
+      // If we're pointing to something unresolved, be
       // conservative.
       if (bpt->toType()->isPlaceholder())
         return buildDIType(pointerType(voidType()), helper);
diff --git a/bridge/go-llvm-typemanager.h b/bridge/go-llvm-typemanager.h
index b2a9660..c3e0b1b 100644
--- a/bridge/go-llvm-typemanager.h
+++ b/bridge/go-llvm-typemanager.h
@@ -262,8 +262,6 @@
 
   llvm::DIType *buildStructDIType(BStructType *bst, DIBuildHelper &helper);
 
-  llvm::DIType *buildCircularPointerDIType(Btype *typ, DIBuildHelper &helper);
-
   std::vector<Btyped_identifier>
   sanitizeFields(const std::vector<Btyped_identifier> &fields);
 
@@ -335,13 +333,8 @@
   std::unordered_map<Btype *, Btype *> circularPointerTypeMap_;
 
   // Maps for inserting conversions involving circular pointers.
-  std::unordered_map<Btype *, Btype *> circularConversionLoadMap_;
-  std::unordered_map<Btype *, Btype *> circularConversionAddrMap_;
-
-  // For storing the pointers involved in a circular pointer type loop.
-  // Temporary; filled in only during processing of the loop.
-  typedef std::pair<Btype *, Btype *> btpair;
-  std::vector<btpair> circularPointerLoop_;
+  std::unordered_map<llvm::Type *, Btype *> circularConversionLoadMap_;
+  std::unordered_map<llvm::Type *, Btype *> circularConversionAddrMap_;
 
   // Set of top-level circular function types.
   std::unordered_set<Btype *> circularFunctionPlaceholderTypes_;
diff --git a/unittests/BackendCore/BackendFcnTests.cpp b/unittests/BackendCore/BackendFcnTests.cpp
index 23765c4..160f0ea 100644
--- a/unittests/BackendCore/BackendFcnTests.cpp
+++ b/unittests/BackendCore/BackendFcnTests.cpp
@@ -195,7 +195,7 @@
   // type P *P
   Btype *cpht = be->placeholder_pointer_type("ph", loc, false);
   Btype *cpt = be->circular_pointer_type(cpht, false);
-  be->set_placeholder_pointer_type(cpht, cpt);
+  be->set_placeholder_pointer_type(cpht, be->pointer_type(cpt));
 
   // struct A { f2 bool, fn *A }
   Btype *php = be->placeholder_pointer_type("ph", loc, false);
diff --git a/unittests/BackendCore/BackendPointerExprTests.cpp b/unittests/BackendCore/BackendPointerExprTests.cpp
index 5b0297e..285dc42 100644
--- a/unittests/BackendCore/BackendPointerExprTests.cpp
+++ b/unittests/BackendCore/BackendPointerExprTests.cpp
@@ -248,7 +248,8 @@
   // Create circular pointer type
   Btype *pht = be->placeholder_pointer_type("ph", loc, false);
   Btype *cpt = be->circular_pointer_type(pht, false);
-  be->set_placeholder_pointer_type(pht, cpt);
+  Btype *pcpt = be->pointer_type(cpt);
+  be->set_placeholder_pointer_type(pht, pcpt);
   EXPECT_EQ(pht->type(), cpt->type());
 
   // Local vars
@@ -325,18 +326,18 @@
   %icmp.0 = icmp eq %CPT.0* %cpv1.ld.0, %.ld.0
   %zext.0 = zext i1 %icmp.0 to i8
   store i8 %zext.0, i8* %b1
-  %cast.3 = bitcast %CPT.0** %cpv1 to %CPT.0*
   %cpv2.ld.1 = load %CPT.0*, %CPT.0** %cpv2
-  %icmp.1 = icmp eq %CPT.0* %cast.3, %cpv2.ld.1
+  %cast.3 = bitcast %CPT.0* %cpv2.ld.1 to %CPT.0**
+  %icmp.1 = icmp eq %CPT.0** %cpv1, %cast.3
   %zext.1 = zext i1 %icmp.1 to i8
   store i8 %zext.1, i8* %b2
   %cpv1.ld.1 = load %CPT.0*, %CPT.0** %cpv1
-  %cast.5 = bitcast %CPT.0** %cpv2 to %CPT.0***
-  %cpv2.ld.2 = load %CPT.0**, %CPT.0*** %cast.5
-  %cast.6 = bitcast %CPT.0** %cpv2.ld.2 to %CPT.0***
-  %deref.ld.0 = load %CPT.0**, %CPT.0*** %cast.6
-  %cast.7 = bitcast %CPT.0** %deref.ld.0 to %CPT.0***
-  %deref.ld.1 = load %CPT.0**, %CPT.0*** %cast.7
+  %cast.4 = bitcast %CPT.0** %cpv2 to %CPT.0***
+  %cpv2.ld.2 = load %CPT.0**, %CPT.0*** %cast.4
+  %cast.5 = bitcast %CPT.0** %cpv2.ld.2 to %CPT.0***
+  %deref.ld.0 = load %CPT.0**, %CPT.0*** %cast.5
+  %cast.6 = bitcast %CPT.0** %deref.ld.0 to %CPT.0***
+  %deref.ld.1 = load %CPT.0**, %CPT.0*** %cast.6
   %.ld.1 = load %CPT.0*, %CPT.0** %deref.ld.1
   %icmp.2 = icmp eq %CPT.0* %cpv1.ld.1, %.ld.1
   %zext.2 = zext i1 %icmp.2 to i8
@@ -372,8 +373,9 @@
   Btype *pht1 = be->placeholder_pointer_type("ph1", loc, false);
   Btype *pht2 = be->placeholder_pointer_type("ph2", loc, false);
   Btype *cpt = be->circular_pointer_type(pht1, false);
-  be->set_placeholder_pointer_type(pht2, be->pointer_type(cpt));
-  be->set_placeholder_pointer_type(pht1, cpt);
+  Btype *pcpt = be->pointer_type(cpt);
+  be->set_placeholder_pointer_type(pht2, pcpt);
+  be->set_placeholder_pointer_type(pht1, be->pointer_type(pcpt));
 
   // Local vars
   Bvariable *cpv1 = h.mkLocal("x", pht1);