[PATCH] D15982: [rs4gc] Optionally directly relocated vector of pointers

[Philip Reames via llvm-commits]

reames added a comment.

In http://reviews.llvm.org/D15982#322785, @mjacob wrote:

> Maybe you could move the caching to the lambda. However I'm fine with it if you had good reasons not to do so.


I generally find that having the generation function separate from the caching layer leads to fewer bugs.  Maybe it's just me, but I've seen and written a *lot* of bugs where something didn't get cached correctly.  I like to keep that portion of the code as simple and obviously correct as possible.  I could use a second layer of lambda, but that seemed like a overkill in this situation.


================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1322-1327
@@ -1321,6 +1321,8 @@
+  
   // All gc_relocate are set to i8 addrspace(1)* type. We originally generated
   // unique declarations for each pointer type, but this proved problematic
   // because the intrinsic mangling code is incomplete and fragile.  Since
   // we're moving towards a single unified pointer type anyways, we can just
   // cast everything to an i8* of the right address space.  A bitcast is added
   // later to convert gc_relocate to the actual value's type. 
+  auto getGCRelocateDecl = [&] (Type *Ty) {
----------------
mjacob wrote:
> This comment is a bit inaccurate now.
Will tweak before submission.


http://reviews.llvm.org/D15982