[PATCH] D25464: [NFC] Loop Versioning for LICM code clean up

silviu.baranga@arm.com via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 12 11:03:11 PDT 2016


sbaranga added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:305
-    DEBUG(dbgs() << "    Alias tracker type safety failed!\n");
-    return false;
   }
----------------
eastig wrote:
> It's strange checking. It was questioned in https://reviews.llvm.org/D9151. An answer was: 
> 
> > Actually this is a pre-condition in LICM’s “promoteLoopAccessesToScalars” where it expects all pointers in alias should have same type.
> > To confirm same behaviour we added this check.
> 
> Yes, type checking is needed for AliasSet when all pointers must alias. This is done by LICM. In case of AliasSet whose pointers may alias we can have aliased pointers of different types, e.g. char * and char ** which might be aliased and they have different types.
I agree, this does look strange (and it might not be a NFC), I think either Ashutosh or Adam should comment on this.


================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:524
-  // Initial allocation
-  CurAST = new AliasSetTracker(*AA);
 
----------------
eastig wrote:
> This is not safe. Replaced with unique_ptr.
Why is this not safe? It looks like if we don't use use unique_ptr we end up with less memory usage (unique_ptr won't free the memory until LoopVersioningLICMs destructor gets called or until we run on another loop).


https://reviews.llvm.org/D25464





More information about the llvm-commits mailing list